Skip to content

Commit

Permalink
Merge pull request #72 from r-lib/call-in-coerce-functions
Browse files Browse the repository at this point in the history
Add call argument in coerce functions
  • Loading branch information
EmilHvitfeldt authored Sep 17, 2024
2 parents 782bdbd + 0c19ade commit e124805
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 16 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

* Fixed bug where `coerce_to_sparse_data_frame()` and `coerce_to_sparse_tibble()` didn't work with matrices with fully sparse columns. (#69)

* All coerce functions have received a `call` argument. (#72)

# sparsevctrs 0.1.0

* Initial CRAN submission.
35 changes: 23 additions & 12 deletions R/coerce.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#' [Matrix::sparseMatrix()].
#'
#' @param x a data frame or tibble with sparse columns.
#' @inheritParams rlang::args_error_context
#'
#' @details
#' No checking is currently do to `x` to determine whether it contains sparse
Expand All @@ -22,22 +23,26 @@
#' res <- coerce_to_sparse_matrix(sparse_tbl)
#' res
#' @export
coerce_to_sparse_matrix <- function(x) {
coerce_to_sparse_matrix <- function(x, call = rlang::caller_env(0)) {
rlang::check_installed("Matrix")

if (!inherits(x, "data.frame")) {
cli::cli_abort(
"{.arg x} must be a {.cls data.frame}, not {.obj_type_friendly {x}}."
"{.arg x} must be a {.cls data.frame}, not {.obj_type_friendly {x}}.",
call = call
)
}

if (!all(vapply(x, is.numeric, logical(1)))) {
offenders <- which(!vapply(x, is.numeric, logical(1)))
offenders <- names(x)[offenders]
cli::cli_abort(c(
x = "All columns of {.arg x} must be numeric.",
i = "Non-numeric columns: {.field {offenders}}."
))
cli::cli_abort(
c(
x = "All columns of {.arg x} must be numeric.",
i = "Non-numeric columns: {.field {offenders}}."
),
call = call
)
}

if (!any(vapply(x, is_sparse_numeric, logical(1)))) {
Expand Down Expand Up @@ -76,6 +81,7 @@ coerce_to_sparse_matrix <- function(x) {
#' Turning a sparse matrix into a tibble.
#'
#' @param x sparse matrix.
#' @inheritParams rlang::args_error_context
#'
#' @details
#' The only requirement from the sparse matrix is that it contains column names.
Expand All @@ -96,12 +102,13 @@ coerce_to_sparse_matrix <- function(x) {
#' # All columns are sparse
#' vapply(res, is_sparse_vector, logical(1))
#' @export
coerce_to_sparse_tibble <- function(x) {
coerce_to_sparse_tibble <- function(x, call = rlang::caller_env(0)) {
rlang::check_installed("tibble")

if (!any(methods::is(x) == "sparseMatrix")) {
cli::cli_abort(
"{.arg x} must be a {.cls sparseMatrix}, not {.obj_type_friendly {x}}."
"{.arg x} must be a {.cls sparseMatrix}, not {.obj_type_friendly {x}}.",
call = call
)
}

Expand All @@ -112,7 +119,8 @@ coerce_to_sparse_tibble <- function(x) {

if (is.null(colnames(x))) {
cli::cli_abort(
"{.arg x} must have column names."
"{.arg x} must have column names.",
call = call
)
}

Expand All @@ -126,6 +134,7 @@ coerce_to_sparse_tibble <- function(x) {
#' Turning a sparse matrix into a data frame
#'
#' @param x sparse matrix.
#' @inheritParams rlang::args_error_context
#'
#' @details
#' The only requirement from the sparse matrix is that it contains column names.
Expand All @@ -146,10 +155,11 @@ coerce_to_sparse_tibble <- function(x) {
#' # All columns are sparse
#' vapply(res, is_sparse_vector, logical(1))
#' @export
coerce_to_sparse_data_frame <- function(x) {
coerce_to_sparse_data_frame <- function(x, call = rlang::caller_env(0)) {
if (!any(methods::is(x) == "sparseMatrix")) {
cli::cli_abort(
"{.arg x} must be a {.cls sparseMatrix}, not {.obj_type_friendly {x}}."
"{.arg x} must be a {.cls sparseMatrix}, not {.obj_type_friendly {x}}.",
call = call
)
}

Expand All @@ -160,7 +170,8 @@ coerce_to_sparse_data_frame <- function(x) {

if (is.null(colnames(x))) {
cli::cli_abort(
"{.arg x} must have column names."
"{.arg x} must have column names.",
call = call
)
}

Expand Down
7 changes: 6 additions & 1 deletion man/coerce_to_sparse_data_frame.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion man/coerce_to_sparse_matrix.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion man/coerce_to_sparse_tibble.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 24 additions & 0 deletions tests/testthat/_snaps/coerce.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,22 @@
sparsevctrs: Sparse vector materialized
sparsevctrs: Sparse vector materialized

# coerce_to_sparse_matrix() can pass through error call

Code
tmp_fun(1)
Condition
Error in `tmp_fun()`:
! `x` must be a <data.frame>, not a number.

---

Code
tmp_fun(1)
Condition
Error in `tmp_fun()`:
! `x` must be a <sparseMatrix>, not a number.

# coerce_to_sparse_data_frame() errors with no column names

Code
Expand All @@ -55,6 +71,14 @@
Error in `coerce_to_sparse_data_frame()`:
! `x` must be a <sparseMatrix>, not an integer vector.

# coerce_to_sparse_data_frame() can pass through error call

Code
tmp_fun(1)
Condition
Error in `tmp_fun()`:
! `x` must be a <sparseMatrix>, not a number.

# coerce_to_sparse_tibble() errors with no column names

Code
Expand Down
37 changes: 36 additions & 1 deletion tests/testthat/test-coerce.R
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,17 @@ test_that("coerce_to_sparse_matrix() materializes non-zero defaulted columns", {
expect_identical(res, exp)
})

test_that("coerce_to_sparse_matrix() can pass through error call", {
tmp_fun <- function(x) {
coerce_to_sparse_matrix(x, call = rlang::caller_env(0))
}

expect_snapshot(
error = TRUE,
tmp_fun(1)
)
})

### coerce_to_sparse_data_frame ------------------------------------------------

test_that("coerce_to_sparse_data_frame() works", {
Expand Down Expand Up @@ -140,6 +151,17 @@ test_that("coerce_to_sparse_data_frame() errors with wrong input", {
)
})

test_that("coerce_to_sparse_data_frame() can pass through error call", {
tmp_fun <- function(x) {
coerce_to_sparse_data_frame(x, call = rlang::caller_env(0))
}

expect_snapshot(
error = TRUE,
tmp_fun(1)
)
})

### coerce_to_sparse_tibble ----------------------------------------------------

test_that("coerce_to_sparse_tibble() works", {
Expand Down Expand Up @@ -204,6 +226,19 @@ test_that("coerce_to_sparse_tibble() errors with wrong input", {
)
})

test_that("coerce_to_sparse_matrix() can pass through error call", {
tmp_fun <- function(x) {
coerce_to_sparse_tibble(x, call = rlang::caller_env(0))
}

expect_snapshot(
error = TRUE,
tmp_fun(1)
)
})

### .sparse_matrix_to_list -----------------------------------------------------

test_that(".sparse_matrix_to_list() handles fully sparse columns (#69)", {
skip_if_not_installed("Matrix")

Expand All @@ -220,4 +255,4 @@ test_that(".sparse_matrix_to_list() handles fully sparse columns (#69)", {
coerce_to_sparse_data_frame(x_mat_sparse),
x_df
)
})
})

0 comments on commit e124805

Please sign in to comment.