Skip to content

Commit

Permalink
Merge pull request #1413 from tidymodels/sparse-arg-error
Browse files Browse the repository at this point in the history
  • Loading branch information
EmilHvitfeldt authored Jan 22, 2025
2 parents 2bdc252 + 7dacb51 commit 0375bab
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 2 deletions.
4 changes: 2 additions & 2 deletions R/dummy.R
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +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)
rlang::arg_match0(x$sparse, c("auto", "yes", "no"), arg_nm = "sparse")
check_sparse_arg(x$sparse)

if (length(col_names) > 0) {
## I hate doing this but currently we are going to have
Expand Down Expand Up @@ -302,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(
Expand Down
10 changes: 10 additions & 0 deletions R/sparsevctrs.R
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
17 changes: 17 additions & 0 deletions tests/testthat/test-dummy.R
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
1 change: 1 addition & 0 deletions tests/testthat/test-skipping.R
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down

0 comments on commit 0375bab

Please sign in to comment.