Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

switch to cli in various functions #1244

Merged
merged 10 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

# recipes 1.0.8

## Improvements
Expand Down
7 changes: 2 additions & 5 deletions R/formula.R
Original file line number Diff line number Diff line change
Expand Up @@ -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."
)
}

Expand Down
74 changes: 34 additions & 40 deletions R/misc.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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."
Comment on lines +273 to +277
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't add tests for this, as it is technically deprecated. I opened an issue to properly handle that here #1243

)
)
}
Expand Down Expand Up @@ -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}."
)
}


Expand Down Expand Up @@ -607,30 +607,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)) {
Expand All @@ -643,21 +639,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)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the bug that is mentioned in the PR. $training never worked.

cli::cli_warn(
c(
"!" = "The previous data will be used by {.fn prep}.",
"i" = "the data passed using {.arg training} will be ignored."
EmilHvitfeldt marked this conversation as resolved.
Show resolved Hide resolved
),
call = call
)
}
x <- rec$template
Expand Down Expand Up @@ -779,11 +774,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)
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/_snaps/basics.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
35 changes: 35 additions & 0 deletions tests/testthat/_snaps/misc.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,38 @@
Output
[1] "Error in `step_dummy()`:\nCaused by error in `prep()`:\n! All columns selected for the step should be string, factor, or ordered."

# 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.

52 changes: 52 additions & 0 deletions tests/testthat/_snaps/retraining.md
Original file line number Diff line number Diff line change
@@ -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

27 changes: 27 additions & 0 deletions tests/testthat/test-misc.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
})
21 changes: 12 additions & 9 deletions tests/testthat/test-retraining.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
)
})