From 29d16a7be926190a2c845dd6671e10c017cc8592 Mon Sep 17 00:00:00 2001 From: Emil Hvitfeldt Date: Tue, 21 Jan 2025 15:36:56 -0800 Subject: [PATCH 1/4] refactor out `check_sparse_arg()` --- R/dummy.R | 1 - 1 file changed, 1 deletion(-) diff --git a/R/dummy.R b/R/dummy.R index 854f4aaa9..b90cd7ba1 100644 --- a/R/dummy.R +++ b/R/dummy.R @@ -182,7 +182,6 @@ prep.step_dummy <- function(x, training, info = NULL, ...) { check_type(training[, col_names], types = c("factor", "ordered")) check_bool(x$one_hot, arg = "one_hot") check_function(x$naming, arg = "naming", allow_empty = FALSE) - rlang::arg_match0(x$sparse, c("auto", "yes", "no"), arg_nm = "sparse") if (length(col_names) > 0) { ## I hate doing this but currently we are going to have From 1c256341a87a3b67aab50e94d070d482da3e667f Mon Sep 17 00:00:00 2001 From: Emil Hvitfeldt Date: Tue, 21 Jan 2025 15:41:58 -0800 Subject: [PATCH 2/4] add sparse_is_yes() --- R/sparsevctrs.R | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/R/sparsevctrs.R b/R/sparsevctrs.R index 164fa2e1f..3a976bc80 100644 --- a/R/sparsevctrs.R +++ b/R/sparsevctrs.R @@ -111,3 +111,13 @@ is_sparse_matrix <- function(x) { zeroes / (n_rows * n_cols) } + +check_sparse_arg <- function(x) { + if (!is.null(x)) { + rlang::arg_match0(x, c("auto", "yes", "no"), arg_nm = "sparse") + } +} + +sparse_is_yes <- function(x) { + !is.null(x) && x == "yes" +} \ No newline at end of file From 2be6b7ed6425b84276031f98c14ff339d3dccf57 Mon Sep 17 00:00:00 2001 From: Emil Hvitfeldt Date: Tue, 21 Jan 2025 15:42:06 -0800 Subject: [PATCH 3/4] use and test helper functions --- R/dummy.R | 3 ++- tests/testthat/test-dummy.R | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/R/dummy.R b/R/dummy.R index b90cd7ba1..3ea7c58fb 100644 --- a/R/dummy.R +++ b/R/dummy.R @@ -182,6 +182,7 @@ prep.step_dummy <- function(x, training, info = NULL, ...) { check_type(training[, col_names], types = c("factor", "ordered")) check_bool(x$one_hot, arg = "one_hot") check_function(x$naming, arg = "naming", allow_empty = FALSE) + check_sparse_arg(x$sparse) if (length(col_names) > 0) { ## I hate doing this but currently we are going to have @@ -301,7 +302,7 @@ bake.step_dummy <- function(object, new_data, ...) { ordered = is_ordered ) - if (object$sparse == "yes") { + if (sparse_is_yes(object$sparse)) { current_contrast <- getOption("contrasts")[is_ordered + 1] if (!current_contrast %in% c("contr.treatment", "contr_one_hot")) { cli::cli_abort( diff --git a/tests/testthat/test-dummy.R b/tests/testthat/test-dummy.R index 1175d89c9..e51c8a3c4 100644 --- a/tests/testthat/test-dummy.R +++ b/tests/testthat/test-dummy.R @@ -382,6 +382,23 @@ test_that("sparse = 'yes' errors on unsupported contrasts", { ) }) +test_that("sparse argument is backwards compatible", { + dat <- tibble(x = c(letters)) + rec <- recipe(~ ., data = dat) %>% + step_dummy(x) %>% + prep() + + exp <- bake(rec, dat) + + # Simulate old recipe + rec$steps[[1]]$sparse <- NULL + + expect_identical( + bake(rec, dat), + exp + ) +}) + test_that(".recipes_toggle_sparse_args works", { rec <- recipe(~., iris) %>% step_dummy(all_nominal_predictors()) From 7dacb51d032a16510be5ef6c9cc715d28c9421a1 Mon Sep 17 00:00:00 2001 From: Emil Hvitfeldt Date: Tue, 21 Jan 2025 15:57:28 -0800 Subject: [PATCH 4/4] add check_sparse_arg to test-skipping --- tests/testthat/test-skipping.R | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/testthat/test-skipping.R b/tests/testthat/test-skipping.R index 838dd20fe..7a8a8d34a 100644 --- a/tests/testthat/test-skipping.R +++ b/tests/testthat/test-skipping.R @@ -46,6 +46,7 @@ test_that("check existing steps for `skip` arg", { step_check <- step_check[step_check != "check_role_requirements"] step_check <- step_check[step_check != "check_bake_role_requirements"] step_check <- step_check[step_check != "check_step_check_args"] + step_check <- step_check[step_check != "check_sparse_arg"] # R/import-standalone-types-check.R step_check <- step_check[step_check != "check_bool"]