Merge pull request #1244 from tidymodels/cli-misc

tidymodels · Oct 13, 2023 · 254bf41 · 254bf41
2 parents 26a7ed8 + a102201
commit 254bf41
Show file tree

Hide file tree

Showing 10 changed files with 177 additions and 56 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -8,6 +8,8 @@
 
 * It is now documented that `step_spline_b()` can be made periodic. (#1223)
 
+* `prep()` now correctly throws a warning when `training` argument is set when prepping a prepped recipe, telling the user that it will be ignored. (#1244)
+
 * When errors are thrown about wrongly typed input to steps, the offending variables and their types are now listed. (#1217)
 
 # recipes 1.0.8

diff --git a/R/formula.R b/R/formula.R
@@ -18,11 +18,8 @@
 #' @export
 formula.recipe <- function(x, ...) {
   if (!fully_trained(x)) {
-    rlang::abort(
-      paste0(
-        "The recipe must be prepped before the ",
-        "formula can be computed."
-      )
+    cli::cli_abort(
+      "The recipe must be prepped before the formula can be computed."
     )
   }
 

diff --git a/R/misc.R b/R/misc.R
@@ -83,7 +83,7 @@ get_rhs_vars <- function(formula, data, no_lhs = FALSE) {
 #' @export
 names0 <- function(num, prefix = "x") {
   if (num < 1) {
-    rlang::abort("`num` should be > 0.")
+    cli::cli_abort("{.arg num} should be > 0.")
   }
   ind <- format(seq_len(num))
   ind <- gsub(" ", "0", ind)
@@ -270,10 +270,11 @@ merge_term_info <- function(.new, .old) {
 ellipse_check <- function(...) {
   terms <- quos(...)
   if (is_empty(terms)) {
-    rlang::abort(
-      paste0(
-        "Please supply at least one variable specification.",
-        "See ?selections."
+    cli::cli_abort(
+      c(
+        "!" = "Please supply at least one variable specification.",
+        "i" = "See {.help [?selections](recipes::selections)} \\
+              for more information."
       )
     )
   }
@@ -325,10 +326,9 @@ printer <- function(tr_obj = NULL,
 #' @keywords internal
 #' @rdname recipes-internal
 prepare <- function(x, ...) {
-  rlang::abort(paste0(
-    "As of version 0.0.1.9006, used `prep` ",
-    "instead of `prepare`"
-  ))
+  cli::cli_abort(
+    "As of version 0.0.1.9006 please use {.fn prep} instead of {.fn prepare}."
+  )
 }
 
 
@@ -635,30 +635,26 @@ check_nominal_type <- function(x, lvl) {
   invisible(NULL)
 }
 
-check_training_set <- function(x, rec, fresh) {
+check_training_set <- function(x, rec, fresh, call = rlang::caller_env()) {
   # In case a variable has multiple roles
   vars <- unique(rec$var_info$variable)
 
   if (is.null(x)) {
     if (fresh) {
-      rlang::abort(
-        paste0(
-          "A training set must be supplied to the `training` argument ",
-          "when `fresh = TRUE`."
-        )
+      cli::cli_abort(
+        "A training set must be supplied to the {.arg training} argument \\
+        when {.code fresh = TRUE}.",
+        call = call
       )
     }
     x <- rec$template
   } else {
     in_data <- vars %in% colnames(x)
     if (!all(in_data)) {
-      rlang::abort(
-        paste0(
-          "Not all variables in the recipe are present in the supplied ",
-          "training set: ",
-          paste0("'", vars[!in_data], "'", collapse = ", "),
-          "."
-        )
+      cli::cli_abort(
+        "Not all variables in the recipe are present in the supplied training \\
+        set: {.and {.var {vars[!in_data]}}}.",
+        call = call
       )
     }
     if (!is_tibble(x)) {
@@ -671,21 +667,20 @@ check_training_set <- function(x, rec, fresh) {
   steps_trained <- vapply(rec$steps, is_trained, logical(1))
   if (any(steps_trained) & !fresh) {
     if (!rec$retained) {
-      rlang::abort(
-        paste0(
-          "To prep new steps after prepping the original ",
-          "recipe, `retain = TRUE` must be set each time that ",
-          "the recipe is trained."
-        )
+      cli::cli_abort(
+        "To prep new steps after prepping the original recipe, \\
+        {.code retain = TRUE} must be set each time that the recipe is \\
+        trained.",
+        call = call
       )
     }
-    if (!is.null(rec$training)) {
-      rlang::warn(
-        paste0(
-          "The previous data will be used by `prep`; ",
-          "the data passed using `training` will be ",
-          "ignored."
-        )
+    if (!is.null(rec$template)) {
+      cli::cli_warn(
+        c(
+          "!" = "The previous data will be used by {.fn prep}.",
+          "i" = "The data passed using {.arg training} will be ignored."
+        ),
+        call = call
       )
     }
     x <- rec$template
@@ -807,11 +802,10 @@ dimred_data <- function(dat) {
 uses_dim_red <- function(x) {
   dr <- inherits(x, "dimRedResult")
   if (dr) {
-    rlang::abort(
-      paste(
-        "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."
-      )
+    cli::cli_abort(
+      "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 {.href [#823](https://github.com/tidymodels/recipes/issues/823)}."
     )
   }
   invisible(NULL)

diff --git a/tests/testthat/_snaps/basics.md b/tests/testthat/_snaps/basics.md
@@ -36,7 +36,7 @@
       prepare(recipe(HHV ~ ., data = biomass), training = biomass)
     Condition
       Error in `prepare()`:
-      ! As of version 0.0.1.9006, used `prep` instead of `prepare`
+      ! As of version 0.0.1.9006 please use `prep()` instead of `prepare()`.
 
 # bake without prep
 

diff --git a/tests/testthat/_snaps/formula.md b/tests/testthat/_snaps/formula.md
@@ -1,3 +1,12 @@
+# is trained?
+
+    Code
+      rec5 <- prep(rec4, training = iris)
+    Condition
+      Warning in `prep()`:
+      ! The previous data will be used by `prep()`.
+      i The data passed using `training` will be ignored.
+
 # bad args
 
     Code

diff --git a/tests/testthat/_snaps/misc.md b/tests/testthat/_snaps/misc.md
@@ -29,3 +29,38 @@
     Output
       [1] "Error in `step_dummy()`:\nCaused by error in `prep()`:\nx All columns selected for the step should be string, factor, or ordered.\n* 11 double variables found: `mpg`, `cyl`, `disp`, `hp`, ..."
 
+# check_training_set errors are thrown
+
+    Code
+      recipe(~., data = mtcars) %>% prep(fresh = TRUE)
+    Condition
+      Error in `prep()`:
+      ! A training set must be supplied to the `training` argument when `fresh = TRUE`.
+
+---
+
+    Code
+      recipe(~., data = mtcars) %>% prep(mtcars[, 1:2], fresh = TRUE)
+    Condition
+      Error in `prep()`:
+      ! Not all variables in the recipe are present in the supplied training set: `disp`, `hp`, `drat`, `wt`, `qsec`, `vs`, `am`, `gear`, and `carb`.
+
+---
+
+    Code
+      recipe(~., data = mtcars) %>% step_center(disp) %>% prep(retain = FALSE) %>%
+        prep(mtcars, fresh = FALSE)
+    Condition
+      Error in `prep()`:
+      ! To prep new steps after prepping the original recipe, `retain = TRUE` must be set each time that the recipe is trained.
+
+---
+
+    Code
+      tmp <- recipe(~., data = mtcars) %>% step_center(disp) %>% prep() %>% prep(
+        mtcars)
+    Condition
+      Warning in `prep()`:
+      ! The previous data will be used by `prep()`.
+      i The data passed using `training` will be ignored.
+
diff --git a/tests/testthat/_snaps/retraining.md b/tests/testthat/_snaps/retraining.md
@@ -0,0 +1,52 @@
+# training in stages
+
+    Code
+      no_sulfur_trained <- prep(no_sulfur)
+    Condition
+      Warning in `prep()`:
+      ! The previous data will be used by `prep()`.
+      i The data passed using `training` will be ignored.
+
+---
+
+    Code
+      sequentially <- prep(scale_last)
+    Condition
+      Warning in `prep()`:
+      ! The previous data will be used by `prep()`.
+      i The data passed using `training` will be ignored.
+
+---
+
+    Code
+      in_stages_trained <- prep(in_stages)
+    Condition
+      Warning in `prep()`:
+      ! The previous data will be used by `prep()`.
+      i The data passed using `training` will be ignored.
+
+---
+
+    Code
+      rec %>% step_center(carbon, hydrogen, oxygen, nitrogen, sulfur) %>% prep(
+        training = biomass) %>% step_rm(sulfur) %>% prep(training = biomass)
+    Condition
+      Warning in `prep()`:
+      ! The previous data will be used by `prep()`.
+      i The data passed using `training` will be ignored.
+    Message
+      
+      -- Recipe ----------------------------------------------------------------------
+      
+      -- Inputs 
+      Number of variables by role
+      outcome:   1
+      predictor: 5
+      
+      -- Training information 
+      Training data contained 536 data points and no incomplete rows.
+      
+      -- Operations 
+      * Centering for: carbon, hydrogen, oxygen, nitrogen, sulfur | Trained
+      * Variables removed: sulfur | Trained
+
diff --git a/tests/testthat/test-formula.R b/tests/testthat/test-formula.R
@@ -18,7 +18,9 @@ test_that("is trained?", {
   rec4 <- rec3 %>% step_scale(all_numeric())
   expect_false(fully_trained(rec4))
 
-  rec5 <- prep(rec4, training = iris)
+  expect_snapshot(
+    rec5 <- prep(rec4, training = iris)
+  )
   expect_true(fully_trained(rec5))
 })
 

diff --git a/tests/testthat/test-misc.R b/tests/testthat/test-misc.R
@@ -28,3 +28,30 @@ test_that("conditionMessage method for recipes errors works", {
 
   expect_snapshot(conditionMessage(attr(res, "condition")))
 })
+
+test_that("check_training_set errors are thrown", {
+  expect_snapshot(
+    error = TRUE,
+    recipe(~., data = mtcars) %>% prep(fresh = TRUE)
+  )
+
+  expect_snapshot(
+    error = TRUE,
+    recipe(~., data = mtcars) %>% prep(mtcars[, 1:2], fresh = TRUE)
+  )
+
+  expect_snapshot(
+    error = TRUE,
+    recipe(~., data = mtcars) %>%
+      step_center(disp) %>%
+      prep(retain = FALSE) %>%
+      prep(mtcars, fresh = FALSE)
+  )
+
+  expect_snapshot(
+    tmp <- recipe(~., data = mtcars) %>%
+      step_center(disp) %>%
+      prep() %>%
+      prep(mtcars)
+  )
+})
diff --git a/tests/testthat/test-retraining.R b/tests/testthat/test-retraining.R
@@ -24,19 +24,23 @@ test_that("training in stages", {
 
   no_sulfur <- center_first_trained %>%
     step_rm(sulfur)
-  no_sulfur_trained <-
-    prep(no_sulfur)
+
+  expect_snapshot(
+    no_sulfur_trained <- prep(no_sulfur)
+  )
 
   scale_last <- no_sulfur_trained %>%
     step_scale(carbon, hydrogen, oxygen, nitrogen)
-  sequentially <- prep(scale_last)
-
+  expect_snapshot(
+    sequentially <- prep(scale_last)
+  )
 
   in_stages <- center_first_trained %>%
     step_rm(sulfur) %>%
     step_scale(carbon, hydrogen, oxygen, nitrogen)
-  in_stages_trained <-
-    prep(in_stages)
+  expect_snapshot(
+    in_stages_trained <- prep(in_stages)
+  )
   in_stages_retrained <-
     prep(in_stages, training = biomass, fresh = TRUE)
 
@@ -69,12 +73,11 @@ test_that("training in stages", {
     summary(in_stages_retrained)
   )
 
-  expect_error(
+  expect_snapshot(
     rec %>%
       step_center(carbon, hydrogen, oxygen, nitrogen, sulfur) %>%
       prep(training = biomass) %>%
       step_rm(sulfur) %>%
-      prep(training = biomass),
-    regexp = NA
+      prep(training = biomass)
   )
 })