From 99208a5e9d0dafe0ab070fff1c7b1309f0825769 Mon Sep 17 00:00:00 2001 From: Emil Hvitfeldt Date: Fri, 25 Oct 2024 13:29:25 -0700 Subject: [PATCH] add snapshots for remaining errors --- R/kpca_rbf.R | 2 +- R/misc.R | 5 +- R/recipe.R | 18 +---- R/regex.R | 8 -- tests/testthat/_snaps/basics.md | 88 ++++++++++++++++++++++ tests/testthat/_snaps/extract_parameter.md | 12 +++ tests/testthat/_snaps/kpca_rbf.md | 10 +++ tests/testthat/_snaps/misc.md | 25 ++++++ tests/testthat/_snaps/tune_args.md | 23 ++++++ tests/testthat/_snaps/update.md | 8 ++ tests/testthat/test-basics.R | 73 ++++++++++++++++++ tests/testthat/test-extract_parameter.R | 17 +++++ tests/testthat/test-kpca_rbf.R | 17 +++++ tests/testthat/test-misc.R | 23 ++++++ tests/testthat/test-tune_args.R | 24 ++++++ tests/testthat/test-update.R | 11 ++- 16 files changed, 335 insertions(+), 29 deletions(-) create mode 100644 tests/testthat/_snaps/extract_parameter.md create mode 100644 tests/testthat/_snaps/tune_args.md create mode 100644 tests/testthat/test-extract_parameter.R create mode 100644 tests/testthat/test-tune_args.R diff --git a/R/kpca_rbf.R b/R/kpca_rbf.R index d96afec52..677b71e0e 100644 --- a/R/kpca_rbf.R +++ b/R/kpca_rbf.R @@ -134,7 +134,7 @@ prep.step_kpca_rbf <- function(x, training, info = NULL, ...) { cli::cli_abort(c( x = "Failed with error:", i = as.character(kprc) - ), .internal = TRUE) + )) } } else { kprc <- NULL diff --git a/R/misc.R b/R/misc.R index 87a661344..a907885a8 100644 --- a/R/misc.R +++ b/R/misc.R @@ -99,9 +99,8 @@ get_rhs_vars <- function(formula, data, no_lhs = FALSE) { #' dummy_names("x", substring(after_mm, 2), ordinal = TRUE) #' @export names0 <- function(num, prefix = "x") { - if (num < 1) { - cli::cli_abort("{.arg num} should be > 0.") - } + check_number_whole(num, min = 1) + ind <- format(seq_len(num)) ind <- gsub(" ", "0", ind) paste0(prefix, ind) diff --git a/R/recipe.R b/R/recipe.R index 66e34cd73..4df7986e4 100644 --- a/R/recipe.R +++ b/R/recipe.R @@ -140,8 +140,7 @@ recipe.data.frame <- cli::cli_abort(c( x = "{.arg vars} must have unique values.", - i = "The following values were duplicated:", - "*" = "{.and {offenders}}" + i = "The following values were duplicated: {.and {.field {offenders}}}." )) } if (any(!(vars %in% colnames(x)))) { @@ -149,7 +148,7 @@ recipe.data.frame <- cli::cli_abort(c( x = "The following elements of {.arg vars} are not found in {.arg x}:", - "*" = "{.and {offenders}}" + "*" = "{.and {.field {offenders}}}." )) } @@ -684,7 +683,7 @@ bake.recipe <- function(object, new_data, ..., composition = "tibble") { if (!any(composition == formats)) { cli::cli_abort(c( "x" = "{.arg composition} cannot be {.val {composition}}.", - "i" = "Allowed values are {.or {formats}}." + "i" = "Allowed values are {.or {.val {formats}}}." )) } @@ -693,17 +692,6 @@ bake.recipe <- function(object, new_data, ..., composition = "tibble") { terms <- quos(everything()) } - # In case someone used the deprecated `newdata`: - if (is.null(new_data) || is.null(ncol(new_data))) { - if (any(names(terms) == "newdata")) { - cli::cli_abort( - "Please use {.arg new_data} instead of {.arg newdata} with {.fun bake}." - ) - } else { - cli::cli_abort("Please pass a data set to {.arg new_data}.") - } - } - if (is_sparse_matrix(new_data)) { new_data <- sparsevctrs::coerce_to_sparse_tibble( new_data, diff --git a/R/regex.R b/R/regex.R index 720b37f9d..b89f2903f 100644 --- a/R/regex.R +++ b/R/regex.R @@ -124,14 +124,6 @@ prep.step_regex <- function(x, training, info = NULL, ...) { col_name <- recipes_eval_select(x$terms, training, info) check_type(training[, col_name], types = c("string", "factor", "ordered")) - if (length(col_name) > 1) { - cli::cli_abort(c( - x = "The selector should select at most a single variable.", - i = "The following {length(col_name)} were selected: \\ - {.and {.var {col_name}}}." - )) - } - step_regex_new( terms = x$terms, role = x$role, diff --git a/tests/testthat/_snaps/basics.md b/tests/testthat/_snaps/basics.md index 29c0145d6..376dd3f3e 100644 --- a/tests/testthat/_snaps/basics.md +++ b/tests/testthat/_snaps/basics.md @@ -238,3 +238,91 @@ Error in `as.data.frame.default()`: ! cannot coerce class '"function"' to a data.frame +# bake() error on wrong composition + + Code + recipe(~., data = mtcars) %>% prep() %>% bake(mtcars, composition = "wrong") + Condition + Error in `bake()`: + x `composition` cannot be "wrong". + i Allowed values are "tibble", "dgCMatrix", "matrix", or "data.frame". + +# juice() error on wrong composition + + Code + recipe(~., data = mtcars) %>% prep() %>% juice(composition = "wrong") + Condition + Error in `juice()`: + x `composition` cannot be "wrong". + i Allowed values are "tibble", "dgCMatrix", "matrix", or "data.frame". + +# juice() error if prep(retain = FALSE) + + Code + recipe(~., data = mtcars) %>% prep(retain = FALSE) %>% juice() + Condition + Error in `juice()`: + ! Use `retain = TRUE` in `prep()` to be able to extract the training set. + +# recipe() error with minus in formula + + Code + recipe(~ . - 1, data = mtcars) + Condition + Error in `recipe()`: + x `-` is not allowed in a recipe formula. + i Use `step_rm()` (`?recipes::step_rm()`) instead. + +# recipe() error if vars and roles have different lengths + + Code + recipe(mtcars, vars = c("mpg", "disp"), roles = c("predictor")) + Condition + Error in `recipe()`: + x `vars` and `roles` must have same length. + * `vars` has length 2 + * `roles` has length 1 + +# recipe() error if vars not in data + + Code + recipe(mtcars, vars = c("wrong", "disp-wrong")) + Condition + Error in `recipe()`: + x The following elements of `vars` are not found in `x`: + * wrong and disp-wrong. + +# recipe() error if vars contains duplicates + + Code + recipe(mtcars, vars = c("mpg", "mpg")) + Condition + Error in `recipe()`: + x `vars` must have unique values. + i The following values were duplicated: mpg. + +# recipe() error if vars and roles are used with formula + + Code + recipe(mtcars, ~., vars = c("mpg")) + Condition + Error in `recipe()`: + ! The `vars` argument will be ignored when a formula is used. + +--- + + Code + recipe(mtcars, ~., roles = c("mpg")) + Condition + Error in `recipe()`: + ! The `roles` argument will be ignored when a formula is used. + +# recipe() error for unsupported data types + + Code + recipe(list()) + Condition + Error in `recipe()`: + x `x` should be a data frame, matrix, formula, or tibble. + i `x` is an empty list. + diff --git a/tests/testthat/_snaps/extract_parameter.md b/tests/testthat/_snaps/extract_parameter.md new file mode 100644 index 000000000..a312abc48 --- /dev/null +++ b/tests/testthat/_snaps/extract_parameter.md @@ -0,0 +1,12 @@ +# rethrows error correctly from implementation + + Code + params <- extract_parameter_set_dials(rec) + Condition + Error in `mutate()`: + i In argument: `object = purrr::map(call_info, eval_call_info)`. + Caused by error in `purrr::map()`: + i In index: 1. + Caused by error in `.f()`: + ! Error when calling `num_comp()`: Error in dials::num_comp(range = c(1L, 4L)) : mocked error + diff --git a/tests/testthat/_snaps/kpca_rbf.md b/tests/testthat/_snaps/kpca_rbf.md index 41cb8b233..f5691357d 100644 --- a/tests/testthat/_snaps/kpca_rbf.md +++ b/tests/testthat/_snaps/kpca_rbf.md @@ -27,6 +27,16 @@ ! Name collision occurred. The following variable names already exist: * `kPC1` +# rethrows error correctly from implementation + + Code + recipe(~., data = mtcars) %>% step_kpca_rbf(all_predictors()) %>% prep() + Condition + Error in `step_kpca_rbf()`: + Caused by error in `prep()`: + x Failed with error: + i Error in kernlab::kpca(x = as.matrix(training[, col_names]), features = 5, : mocked error + # bake method errors when needed non-standard role columns are missing Code diff --git a/tests/testthat/_snaps/misc.md b/tests/testthat/_snaps/misc.md index 3fd949ed8..0cfe46036 100644 --- a/tests/testthat/_snaps/misc.md +++ b/tests/testthat/_snaps/misc.md @@ -80,3 +80,28 @@ Error in `recipes:::spline_msg()`: ! craaazzyy {{}}{}{} +# names0() error on non-positive number + + Code + names0(0) + Condition + Error in `names0()`: + ! `num` must be a whole number larger than or equal to 1, not the number 0. + +# ellipse_check() errors on empty selection + + Code + ellipse_check() + Condition + Error in `ellipse_check()`: + ! Please supply at least one variable specification. + i See ?selections (`?recipes::selections()`) for more information. + +--- + + Code + uses_dim_red(x) + Condition + Error in `uses_dim_red()`: + ! Recipes version >= 0.1.17 represents the estimates using a different format. Please recreate this recipe or use version 0.1.16 or less. See issue #823 (). + diff --git a/tests/testthat/_snaps/tune_args.md b/tests/testthat/_snaps/tune_args.md new file mode 100644 index 000000000..035419f5e --- /dev/null +++ b/tests/testthat/_snaps/tune_args.md @@ -0,0 +1,23 @@ +# tune_args() errors on multiple tune()s in same arg + + Code + recipe(~., data = mtcars) %>% step_pca(all_predictors(), num_comp = ~ tune() + + tune()) %>% tune_args() + Condition + Error in `purrr::map()`: + i In index: 1. + Caused by error in `purrr::map_chr()`: + i In index: 1. + i With name: num_comp. + Caused by error in `find_tune_id()`: + ! Only one tunable value is currently allowed per argument. The current argument has: tune() + tune(). + +# tune_tbl() errors on duplicate ids + + Code + tune_tbl(name = c("a", "b"), tunable = c(TRUE, TRUE), id = c("a", "a"), source = c( + "a", "b"), component = c("a", "b"), component_id = c("a", "b"), full = TRUE) + Condition + Error in `tune_tbl()`: + ! There are duplicate id values listed in `tune()`: "a". + diff --git a/tests/testthat/_snaps/update.md b/tests/testthat/_snaps/update.md index 78c775d08..a6a52ddbf 100644 --- a/tests/testthat/_snaps/update.md +++ b/tests/testthat/_snaps/update.md @@ -14,3 +14,11 @@ Error in `validate_not_trained()`: ! To update `step_stp()`, it must not be trained. +# update() errors on duplicate assignments + + Code + update(step, x = 5, x = 6) + Condition + Error in `validate_has_unique_names()`: + ! All of the changes supplied in `...` must be uniquely named. + diff --git a/tests/testthat/test-basics.R b/tests/testthat/test-basics.R index 5da0dac85..51e5e98a9 100644 --- a/tests/testthat/test-basics.R +++ b/tests/testthat/test-basics.R @@ -426,3 +426,76 @@ test_that("data argument is checked in recipe.formula() (#1325)", { recipe(~ ., data = data) ) }) + +test_that("bake() error on wrong composition", { + expect_snapshot( + error = TRUE, + recipe(~., data = mtcars) %>% + prep() %>% + bake(mtcars, composition = "wrong") + ) +}) + +test_that("juice() error on wrong composition", { + expect_snapshot( + error = TRUE, + recipe(~., data = mtcars) %>% + prep() %>% + juice(composition = "wrong") + ) +}) + +test_that("juice() error if prep(retain = FALSE)", { + expect_snapshot( + error = TRUE, + recipe(~., data = mtcars) %>% + prep(retain = FALSE) %>% + juice() + ) +}) + +test_that("recipe() error with minus in formula", { + expect_snapshot( + error = TRUE, + recipe(~ . - 1, data = mtcars) + ) +}) + +test_that("recipe() error if vars and roles have different lengths", { + expect_snapshot( + error = TRUE, + recipe(mtcars, vars = c("mpg", "disp"), roles = c("predictor")) + ) +}) + +test_that("recipe() error if vars not in data", { + expect_snapshot( + error = TRUE, + recipe(mtcars, vars = c("wrong", "disp-wrong")) + ) +}) + +test_that("recipe() error if vars contains duplicates", { + expect_snapshot( + error = TRUE, + recipe(mtcars, vars = c("mpg", "mpg")) + ) +}) + +test_that("recipe() error if vars and roles are used with formula", { + expect_snapshot( + error = TRUE, + recipe(mtcars, ~., vars = c("mpg")) + ) + expect_snapshot( + error = TRUE, + recipe(mtcars, ~., roles = c("mpg")) + ) +}) + +test_that("recipe() error for unsupported data types", { + expect_snapshot( + error = TRUE, + recipe(list()) + ) +}) diff --git a/tests/testthat/test-extract_parameter.R b/tests/testthat/test-extract_parameter.R new file mode 100644 index 000000000..528c14554 --- /dev/null +++ b/tests/testthat/test-extract_parameter.R @@ -0,0 +1,17 @@ +test_that("rethrows error correctly from implementation", { + skip_if_not_installed("dials") + rec <- recipe(~., data = mtcars) %>% + step_pca(all_predictors(), num_comp = hardhat::tune()) + + local_mocked_bindings( + .package = "dials", + num_comp = function(...) { + cli::cli_abort("mocked error") + } + ) + + expect_snapshot( + error = TRUE, + params <- extract_parameter_set_dials(rec) + ) +}) diff --git a/tests/testthat/test-kpca_rbf.R b/tests/testthat/test-kpca_rbf.R index b3ef7947c..c8b27ca44 100644 --- a/tests/testthat/test-kpca_rbf.R +++ b/tests/testthat/test-kpca_rbf.R @@ -96,6 +96,23 @@ test_that("Do nothing for num_comps = 0 and keep_original_cols = FALSE (#1152)", expect_identical(res, tibble::as_tibble(mtcars)) }) +test_that("rethrows error correctly from implementation", { + skip_if_not_installed("kernlab") + + local_mocked_bindings( + .package = "kernlab", + kpca = function(...) { + cli::cli_abort("mocked error") + } + ) + expect_snapshot( + error = TRUE, + recipe(~ ., data = mtcars) %>% + step_kpca_rbf(all_predictors()) %>% + prep() + ) +}) + # Infrastructure --------------------------------------------------------------- test_that("bake method errors when needed non-standard role columns are missing", { diff --git a/tests/testthat/test-misc.R b/tests/testthat/test-misc.R index 2b61e3888..843dc645f 100644 --- a/tests/testthat/test-misc.R +++ b/tests/testthat/test-misc.R @@ -119,3 +119,26 @@ test_that("spline error messages", { error = TRUE ) }) + +test_that("names0() error on non-positive number", { + expect_snapshot( + error = TRUE, + names0(0) + ) +}) + +test_that("ellipse_check() errors on empty selection", { + expect_snapshot( + error = TRUE, + ellipse_check() + ) +}) + +test_that("ellipse_check() errors on empty selection", { + x <- 2 + class(x) <- "dimRedResult" + expect_snapshot( + error = TRUE, + uses_dim_red(x) + ) +}) diff --git a/tests/testthat/test-tune_args.R b/tests/testthat/test-tune_args.R new file mode 100644 index 000000000..ff0fec7cc --- /dev/null +++ b/tests/testthat/test-tune_args.R @@ -0,0 +1,24 @@ +test_that("tune_args() errors on multiple tune()s in same arg", { + expect_snapshot( + error = TRUE, + recipe(~., data = mtcars) %>% + step_pca(all_predictors(), num_comp = ~tune() + tune()) %>% + tune_args() + ) +}) + +test_that("tune_tbl() errors on duplicate ids", { + expect_snapshot( + error = TRUE, + tune_tbl( + name = c("a", "b"), + tunable = c(TRUE, TRUE), + id = c("a", "a"), + source = c("a", "b"), + component = c("a", "b"), + component_id = c("a", "b"), + full = TRUE + ) + ) +}) + diff --git a/tests/testthat/test-update.R b/tests/testthat/test-update.R index a2d469201..71d5e12fa 100644 --- a/tests/testthat/test-update.R +++ b/tests/testthat/test-update.R @@ -2,8 +2,6 @@ test_that("can update a step", { stp_4 <- recipes::step("stp", x = 4, trained = FALSE) stp_5 <- recipes::step("stp", x = 5, trained = FALSE) - update(stp_4, x = 5) - expect_equal(update(stp_4, x = 5), stp_5) }) @@ -22,3 +20,12 @@ test_that("cannot update trained steps", { update(stp, x = 5) ) }) + +test_that("update() errors on duplicate assignments", { + step <- recipes::step("stp", x = 4, trained = FALSE) + + expect_snapshot( + error = TRUE, + update(step, x = 5, x = 6) + ) +})