From 003039f18cb8fe2dc9120e3d3dbc6ab677c64c7d Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Mon, 21 Oct 2024 10:07:48 +0100 Subject: [PATCH 01/36] update case-weights.R --- R/case-weights.R | 10 +++++----- tests/testthat/_snaps/case-weights.md | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/R/case-weights.R b/R/case-weights.R index 762c73ac..a631d843 100644 --- a/R/case-weights.R +++ b/R/case-weights.R @@ -31,7 +31,7 @@ importance_weights <- function(x) { x <- vec_cast_named(x, to = double(), x_arg = "x") if (any(x < 0, na.rm = TRUE)) { - abort("`x` can't contain negative weights.") + cli::cli_abort("{.arg x} can't contain negative weights.") } new_importance_weights(x) @@ -58,7 +58,7 @@ importance_weights <- function(x) { #' new_importance_weights(c(1.5, 2.3, 10)) new_importance_weights <- function(x = double(), ..., class = character()) { if (!is.double(x)) { - abort("`x` must be a double vector.") + cli::cli_abort("{.arg x} must be a double vector.") } new_case_weights( @@ -153,7 +153,7 @@ frequency_weights <- function(x) { x <- vec_cast_named(x, to = integer(), x_arg = "x") if (any(x < 0L, na.rm = TRUE)) { - abort("`x` can't contain negative weights.") + cli::cli_abort("{.arg x} can't contain negative weights.") } new_frequency_weights(x) @@ -180,7 +180,7 @@ frequency_weights <- function(x) { #' new_frequency_weights(1:5) new_frequency_weights <- function(x = integer(), ..., class = character()) { if (!is.integer(x)) { - abort("`x` must be an integer vector.") + cli::cli_abort("{.arg x} must be an integer vector.") } new_case_weights( @@ -265,7 +265,7 @@ vec_ptype_abbr.hardhat_frequency_weights <- function(x, ...) { #' new_case_weights(1:5, class = "my_weights") new_case_weights <- function(x, ..., class) { if (!is.integer(x) && !is.double(x)) { - abort("`x` must be an integer or double vector.") + cli::cli_abort("{.arg x} must be {.cls integer} or {.cls double} vector.") } new_vctr( diff --git a/tests/testthat/_snaps/case-weights.md b/tests/testthat/_snaps/case-weights.md index 102fe87b..63b23296 100644 --- a/tests/testthat/_snaps/case-weights.md +++ b/tests/testthat/_snaps/case-weights.md @@ -61,5 +61,5 @@ new_case_weights("x", class = "subclass") Condition Error in `new_case_weights()`: - ! `x` must be an integer or double vector. + ! `x` must be or vector. From fd9247e41681069437ee3c3d9c29054e522ea35d Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Mon, 21 Oct 2024 10:09:51 +0100 Subject: [PATCH 02/36] update constructor.R --- R/constructor.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/constructor.R b/R/constructor.R index 8a22aae4..6c6b7870 100644 --- a/R/constructor.R +++ b/R/constructor.R @@ -67,15 +67,15 @@ new_scalar <- function(elems, ..., class = character()) { check_elems <- function(elems) { if (!is.list(elems) || length(elems) == 0) { - abort("`elems` must be a list of length 1 or greater.") + cli::cli_abort("{.arg elems} must be a list of length 1 or greater.") } if (!has_unique_names(elems)) { - abort("`elems` must have unique names.") + cli::cli_abort("{.arg elems} must have unique names.") } if (!identical(names(attributes(elems)), "names")) { - abort("`elems` must have no attributes (apart from names).") + cli::cli_abort("{.arg elems} must have no attributes (apart from names).") } invisible(elems) From 95c9f53e548b4f3bd88791af7c0858bc8686cbcb Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Mon, 21 Oct 2024 10:10:56 +0100 Subject: [PATCH 03/36] update encoding.R --- R/encoding.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/encoding.R b/R/encoding.R index f3587f18..92921c43 100644 --- a/R/encoding.R +++ b/R/encoding.R @@ -31,7 +31,7 @@ #' fct_encode_one_hot(factor(sample(letters[1:4], 10, TRUE))) fct_encode_one_hot <- function(x) { if (!is.factor(x)) { - abort("`x` must be a factor.") + cli::cli_abort("{.arg x} must be a factor.") } row_names <- names(x) @@ -44,7 +44,7 @@ fct_encode_one_hot <- function(x) { x <- unclass(x) if (vec_any_missing(x)) { - abort("`x` can't contain missing values.") + cli::cli_abort("{.arg x} can't contain missing values.") } out <- matrix(0L, nrow = n_rows, ncol = n_cols, dimnames = dim_names) From f4038dcf5e85ae9c5f5eda594f3554e2f3886439 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Mon, 21 Oct 2024 10:14:20 +0100 Subject: [PATCH 04/36] Update forge.R --- R/forge.R | 6 ++---- tests/testthat/_snaps/forge.md | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/R/forge.R b/R/forge.R index b903711f..06545428 100644 --- a/R/forge.R +++ b/R/forge.R @@ -70,7 +70,7 @@ forge <- function(new_data, blueprint, ..., outcomes = FALSE) { #' @export forge.default <- function(new_data, blueprint, ..., outcomes = FALSE) { - glubort("The class of `new_data`, '{class1(new_data)}', is not recognized.") + cli::cli_abort("No {.fn forge} method provided for {.obj_type_friendly {new_data}} object.") } #' @export @@ -140,7 +140,5 @@ run_forge.default <- function(blueprint, new_data, ..., outcomes = FALSE) { - class <- class(blueprint)[[1L]] - message <- glue("No `run_forge()` method provided for an object of type <{class}>.") - abort(message) + cli::cli_abort("No {.fn run_forge} method provided for {.obj_type_friendly {blueprint}} object.") } diff --git a/tests/testthat/_snaps/forge.md b/tests/testthat/_snaps/forge.md index 621dcfb4..86af191f 100644 --- a/tests/testthat/_snaps/forge.md +++ b/tests/testthat/_snaps/forge.md @@ -4,5 +4,5 @@ run_forge(1) Condition Error in `run_forge()`: - ! No `run_forge()` method provided for an object of type . + ! No `run_forge()` method provided for a number object. From b55f6f637fab7631ae5bec5fa079076e67a4c5c7 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Wed, 23 Oct 2024 14:38:56 +0100 Subject: [PATCH 05/36] more snapshot updates for `forge.default()` --- tests/testthat/_snaps/forge-formula.md | 4 ++-- tests/testthat/_snaps/forge-xy.md | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/testthat/_snaps/forge-formula.md b/tests/testthat/_snaps/forge-formula.md index 695a321d..d534c27b 100644 --- a/tests/testthat/_snaps/forge-formula.md +++ b/tests/testthat/_snaps/forge-formula.md @@ -20,7 +20,7 @@ forge("hi", x1$blueprint) Condition Error in `forge()`: - ! The class of `new_data`, 'character', is not recognized. + ! No `forge()` method provided for a string object. --- @@ -28,7 +28,7 @@ forge("hi", x2$blueprint) Condition Error in `forge()`: - ! The class of `new_data`, 'character', is not recognized. + ! No `forge()` method provided for a string object. # missing predictor columns fail appropriately diff --git a/tests/testthat/_snaps/forge-xy.md b/tests/testthat/_snaps/forge-xy.md index 1a6ed11a..08eaba4d 100644 --- a/tests/testthat/_snaps/forge-xy.md +++ b/tests/testthat/_snaps/forge-xy.md @@ -24,7 +24,7 @@ forge("hi", x1$blueprint) Condition Error in `forge()`: - ! The class of `new_data`, 'character', is not recognized. + ! No `forge()` method provided for a string object. --- @@ -32,7 +32,7 @@ forge("hi", x2$blueprint) Condition Error in `forge()`: - ! The class of `new_data`, 'character', is not recognized. + ! No `forge()` method provided for a string object. --- @@ -40,7 +40,7 @@ forge("hi", x3$blueprint) Condition Error in `forge()`: - ! The class of `new_data`, 'character', is not recognized. + ! No `forge()` method provided for a string object. # missing predictor columns fail appropriately From e8d2b6d5d13071316334aaa152fbba3f87110ec0 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Wed, 23 Oct 2024 14:39:39 +0100 Subject: [PATCH 06/36] match the pattern of using using `cli_abort()` directly --- R/blueprint.R | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/R/blueprint.R b/R/blueprint.R index a1872c53..d1933b10 100644 --- a/R/blueprint.R +++ b/R/blueprint.R @@ -289,11 +289,10 @@ check_has_name <- function(x, } } - message <- cli::format_inline( - "{.arg {arg}} must have an element named {.str {name}}." + cli::cli_abort( + "{.arg {arg}} must have an element named {.str {name}}.", + call = call ) - - abort(message, call = call) } # https://github.com/r-lib/rlang/pull/1605 From 4e2a64c86268fd242313fd900a3266303fd98b81 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Wed, 23 Oct 2024 14:42:50 +0100 Subject: [PATCH 07/36] Update `model-matrix.R` --- R/model-matrix.R | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/R/model-matrix.R b/R/model-matrix.R index fed2e19f..47992f48 100644 --- a/R/model-matrix.R +++ b/R/model-matrix.R @@ -178,11 +178,11 @@ model_matrix_one_hot <- function(terms, data) { #' @keywords internal contr_one_hot <- function(n, contrasts = TRUE, sparse = FALSE) { if (sparse) { - warn("`sparse = TRUE` not implemented for `contr_one_hot()`.") + cli::cli_warn("{.code sparse = TRUE} not implemented for {.fn contr_one_hot}.") } if (!contrasts) { - warn("`contrasts = FALSE` not implemented for `contr_one_hot()`.") + cli::cli_warn("{.code contrasts = FALSE} not implemented for {.fn contr_one_hot}.") } if (is.character(n)) { @@ -192,12 +192,12 @@ contr_one_hot <- function(n, contrasts = TRUE, sparse = FALSE) { n <- as.integer(n) if (length(n) != 1L) { - abort("`n` must have length 1 when an integer is provided.") + cli::cli_abort("{.arg n} must have length 1 when an integer is provided.") } names <- as.character(seq_len(n)) } else { - abort("`n` must be a character vector or an integer of size 1.") + cli::cli_abort("{.arg n} must be a character vector or an integer of size 1.") } out <- diag(n) From 859a2eb4623fdd9420a9e26e5ee5741ed18d61bb Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Wed, 23 Oct 2024 14:44:00 +0100 Subject: [PATCH 08/36] Match message from `forge()` --- R/mold.R | 4 +--- tests/testthat/_snaps/mold.md | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/R/mold.R b/R/mold.R index 4e2a1729..b34223c7 100644 --- a/R/mold.R +++ b/R/mold.R @@ -175,7 +175,5 @@ run_mold <- function(blueprint, ...) { #' @export run_mold.default <- function(blueprint, ...) { - class <- class(blueprint)[[1L]] - message <- glue("No `run_mold()` method provided for an object of type <{class}>.") - abort(message) + cli::cli_abort("No {.fn run_mold} method provided for {.obj_type_friendly {blueprint}} object.") } diff --git a/tests/testthat/_snaps/mold.md b/tests/testthat/_snaps/mold.md index 40613108..94dc935c 100644 --- a/tests/testthat/_snaps/mold.md +++ b/tests/testthat/_snaps/mold.md @@ -4,5 +4,5 @@ run_mold(1) Condition Error in `run_mold()`: - ! No `run_mold()` method provided for an object of type . + ! No `run_mold()` method provided for a number object. From f6d069645aa611d384c6a05c74d79ca61489ebe7 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Wed, 23 Oct 2024 15:08:54 +0100 Subject: [PATCH 09/36] "must" instead of "should" - since we error, it really must - "must" is also what the rlang type checkers use --- R/quantile-pred.R | 7 +++++-- tests/testthat/_snaps/quantile-pred.md | 6 +++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/R/quantile-pred.R b/R/quantile-pred.R index fee2e5f6..8a26fff5 100644 --- a/R/quantile-pred.R +++ b/R/quantile-pred.R @@ -57,7 +57,10 @@ new_quantile_pred <- function(values = list(), quantile_levels = double()) { #' @rdname quantile_pred extract_quantile_levels <- function(x) { if (!inherits(x, "quantile_pred")) { - cli::cli_abort("{.arg x} should have class {.cls quantile_pred}.") + cli::cli_abort( + "{.arg x} must be a {.cls quantile_pred} object, not + {.obj_type_friendly {x}}." + ) } attr(x, "quantile_levels") } @@ -169,7 +172,7 @@ check_quantile_levels <- function(levels, call = rlang::caller_env()) { redund <- unique(redund) redund <- signif(redund, digits = 5) cli::cli_abort(c( - "Quantile levels should be unique.", + "Quantile levels must be unique.", i = "The following {cli::qty(length(redund))}value{?s} {?was/were} repeated: {redund}."), call = call diff --git a/tests/testthat/_snaps/quantile-pred.md b/tests/testthat/_snaps/quantile-pred.md index 2a706946..cb2b45e7 100644 --- a/tests/testthat/_snaps/quantile-pred.md +++ b/tests/testthat/_snaps/quantile-pred.md @@ -44,7 +44,7 @@ quantile_pred(matrix(1:20, 5), quantile_levels = c(0.7, 0.7, 0.7)) Condition Error in `quantile_pred()`: - ! Quantile levels should be unique. + ! Quantile levels must be unique. i The following value was repeated: 0.7. --- @@ -53,7 +53,7 @@ quantile_pred(matrix(1:20, 5), quantile_levels = c(rep(0.7, 2), rep(0.8, 3))) Condition Error in `quantile_pred()`: - ! Quantile levels should be unique. + ! Quantile levels must be unique. i The following values were repeated: 0.7 and 0.8. --- @@ -70,7 +70,7 @@ extract_quantile_levels(1:10) Condition Error in `extract_quantile_levels()`: - ! `x` should have class . + ! `x` must be a object, not an integer vector. # quantile_pred formatting From 0fb4c4c0f77595431ba1cdd97991fd7149fe3e8c Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Wed, 23 Oct 2024 15:18:03 +0100 Subject: [PATCH 10/36] Update `table.R` --- R/table.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/table.R b/R/table.R index 2b1778e9..f72f803f 100644 --- a/R/table.R +++ b/R/table.R @@ -103,17 +103,17 @@ weighted_table <- function(..., weights, na_remove = FALSE) { n_args <- length(args) if (n_args == 0L) { - abort("At least one vector must be supplied to `...`.") + cli::cli_abort("At least one vector must be supplied to {.arg ...}.") } if (!all(map_lgl(args, is.factor))) { - abort("All elements of `...` must be factors.") + cli::cli_abort("All elements of {.arg ...} must be factors.") } sizes <- list_sizes(args) size <- sizes[[1L]] if (!all(sizes == size)) { - abort("All elements of `...` must be the same size.") + cli::cli_abort("All elements of {.arg ...} must be the same size.") } weights <- vec_cast(weights, to = double()) From c83c89e547d7e278d57cec621076e2052e7af38d Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Wed, 23 Oct 2024 15:21:42 +0100 Subject: [PATCH 11/36] Update `use.R` --- R/use.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/use.R b/R/use.R index 5b8cd66a..0bbce0d9 100644 --- a/R/use.R +++ b/R/use.R @@ -62,7 +62,7 @@ create_modeling_package <- function(path, check_string(model) if (has_spaces(model)) { - abort("`model` must not contain any spaces.") + cli::cli_abort("{.arg model} must not contain any spaces.") } usethis::create_package(path, fields, open = FALSE) @@ -125,9 +125,9 @@ use_modeling_files_impl <- function(model, prompt_document = TRUE) { check_installed("usethis") check_string(model) - + if (has_spaces(model)) { - abort("`model` must not contain any spaces.") + cli::cli_abort("{.arg model} must not contain any spaces.") } data <- list(model = model) From 570baf1ca8f34a6f50a38bb8f8598472a2e79fc7 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Wed, 23 Oct 2024 15:22:13 +0100 Subject: [PATCH 12/36] Update `util.R` --- R/util.R | 4 ++-- tests/testthat/_snaps/mold-formula.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/R/util.R b/R/util.R index be8442d5..29328d0a 100644 --- a/R/util.R +++ b/R/util.R @@ -14,7 +14,7 @@ simplify_terms <- function(x) { is_terms <- inherits(x, "terms") if (!is_terms) { - abort("`x` must be a terms object") + cli::cli_abort("{.arg x} must be a {.cls terms} object.") } # It removes the environment @@ -64,7 +64,7 @@ get_all_outcomes <- function(formula, data) { outcomes <- all.vars(outcome_formula) if ("." %in% outcomes) { - abort("The left hand side of the formula cannot contain `.`") + cli::cli_abort("The left-hand side of the formula cannot contain {.code .}.") } extra_outcomes <- setdiff(outcomes, names(data)) diff --git a/tests/testthat/_snaps/mold-formula.md b/tests/testthat/_snaps/mold-formula.md index 4dd7d4f3..aa490dfb 100644 --- a/tests/testthat/_snaps/mold-formula.md +++ b/tests/testthat/_snaps/mold-formula.md @@ -318,7 +318,7 @@ mold(. ~ fac_1, example_train) Condition Error in `get_all_outcomes()`: - ! The left hand side of the formula cannot contain `.` + ! The left-hand side of the formula cannot contain `.`. --- @@ -326,7 +326,7 @@ mold(. ~ fac_1, example_train, blueprint = bp) Condition Error in `get_all_outcomes()`: - ! The left hand side of the formula cannot contain `.` + ! The left-hand side of the formula cannot contain `.`. # `blueprint` is validated From 5edd2bf3fe438df141b212526a8f5a35220e68db Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Wed, 23 Oct 2024 15:29:38 +0100 Subject: [PATCH 13/36] Update `validate_column_names()` --- R/validation.R | 6 +----- tests/testthat/_snaps/forge-formula.md | 12 ++++++------ tests/testthat/_snaps/forge-recipe.md | 16 ++++++++-------- tests/testthat/_snaps/forge-xy.md | 8 ++++---- tests/testthat/_snaps/model-offset.md | 2 +- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/R/validation.R b/R/validation.R index 27390461..d1e89583 100644 --- a/R/validation.R +++ b/R/validation.R @@ -495,11 +495,7 @@ validate_column_names <- function(data, original_names) { if (!check$ok) { validate_missing_name_isnt_.outcome(check$missing_names) - missing_names <- glue_quote_collapse(check$missing_names) - - message <- glue("The following required columns are missing: {missing_names}.") - - abort(message) + cli::cli_abort("The required column{?s} {.arg {check$missing_names}} {?is/are} missing.") } invisible(data) diff --git a/tests/testthat/_snaps/forge-formula.md b/tests/testthat/_snaps/forge-formula.md index d534c27b..01ab369e 100644 --- a/tests/testthat/_snaps/forge-formula.md +++ b/tests/testthat/_snaps/forge-formula.md @@ -4,7 +4,7 @@ forge(example_train2, x1$blueprint, outcomes = TRUE) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'fac_1'. + ! The required column `fac_1` is missing. --- @@ -12,7 +12,7 @@ forge(example_train2, x2$blueprint, outcomes = TRUE) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'fac_1'. + ! The required column `fac_1` is missing. # new_data can only be a data frame / matrix @@ -36,7 +36,7 @@ forge(example_train[, 1, drop = FALSE], x1$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'num_2'. + ! The required column `num_2` is missing. --- @@ -44,7 +44,7 @@ forge(example_train[, 1, drop = FALSE], x2$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'num_2'. + ! The required column `num_2` is missing. --- @@ -52,7 +52,7 @@ forge(example_train[, 3, drop = FALSE], x1$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'num_1', 'num_2'. + ! The required columns `num_1` and `num_2` are missing. --- @@ -60,7 +60,7 @@ forge(example_train[, 3, drop = FALSE], x2$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'num_1', 'num_2'. + ! The required columns `num_1` and `num_2` are missing. # novel predictor levels are caught diff --git a/tests/testthat/_snaps/forge-recipe.md b/tests/testthat/_snaps/forge-recipe.md index fde9c1ca..5fb8b049 100644 --- a/tests/testthat/_snaps/forge-recipe.md +++ b/tests/testthat/_snaps/forge-recipe.md @@ -4,7 +4,7 @@ forge(iris2, x1$blueprint, outcomes = TRUE) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'Species'. + ! The required column `Species` is missing. --- @@ -12,7 +12,7 @@ forge(iris2, x2$blueprint, outcomes = TRUE) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'Species'. + ! The required column `Species` is missing. # missing predictor columns fail appropriately @@ -20,7 +20,7 @@ forge(iris[, 1, drop = FALSE], x$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'Sepal.Width'. + ! The required column `Sepal.Width` is missing. --- @@ -28,7 +28,7 @@ forge(iris[, 3, drop = FALSE], x$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'Sepal.Length', 'Sepal.Width'. + ! The required columns `Sepal.Length` and `Sepal.Width` are missing. # novel predictor levels are caught @@ -86,7 +86,7 @@ forge(iris, x$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'Sepal.Width'. + ! The required column `Sepal.Width` is missing. # `NA` roles are treated as extra roles that are required at `forge()` time @@ -94,7 +94,7 @@ forge(iris, x$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'Petal.Length'. + ! The required column `Petal.Length` is missing. # `forge()` is compatible with hardhat 0.2.0 molded blueprints with a basic recipe @@ -102,7 +102,7 @@ forge(new_data, blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'x'. + ! The required column `x` is missing. # `forge()` is compatible with hardhat 0.2.0 molded blueprints with a recipe with a nonstandard role @@ -110,5 +110,5 @@ forge(new_data, blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'id'. + ! The required column `id` is missing. diff --git a/tests/testthat/_snaps/forge-xy.md b/tests/testthat/_snaps/forge-xy.md index 08eaba4d..e32e271f 100644 --- a/tests/testthat/_snaps/forge-xy.md +++ b/tests/testthat/_snaps/forge-xy.md @@ -48,7 +48,7 @@ forge(iris[, 1, drop = FALSE], x1$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'Sepal.Width'. + ! The required column `Sepal.Width` is missing. --- @@ -56,7 +56,7 @@ forge(iris[, 1, drop = FALSE], x2$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'Sepal.Width'. + ! The required column `Sepal.Width` is missing. --- @@ -64,7 +64,7 @@ forge(iris[, 3, drop = FALSE], x1$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'Sepal.Length', 'Sepal.Width'. + ! The required columns `Sepal.Length` and `Sepal.Width` are missing. --- @@ -72,7 +72,7 @@ forge(iris[, 3, drop = FALSE], x2$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'Sepal.Length', 'Sepal.Width'. + ! The required columns `Sepal.Length` and `Sepal.Width` are missing. # novel predictor levels are caught diff --git a/tests/testthat/_snaps/model-offset.md b/tests/testthat/_snaps/model-offset.md index cd93def8..316ca181 100644 --- a/tests/testthat/_snaps/model-offset.md +++ b/tests/testthat/_snaps/model-offset.md @@ -12,5 +12,5 @@ forge(iris2, x$blueprint) Condition Error in `validate_column_names()`: - ! The following required columns are missing: 'Sepal.Length'. + ! The required column `Sepal.Length` is missing. From a14be027517ea16f71ed5b5d35272a0428582901 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Wed, 23 Oct 2024 16:08:18 +0100 Subject: [PATCH 14/36] Update `warn_novel_levels()` --- R/scream.R | 14 ++++----- tests/testthat/_snaps/forge-formula.md | 39 +++++++++++++++++++------- tests/testthat/_snaps/forge-recipe.md | 12 +++++--- tests/testthat/_snaps/forge-xy.md | 9 ++++-- tests/testthat/test-forge-formula.R | 8 ++++++ 5 files changed, 58 insertions(+), 24 deletions(-) diff --git a/R/scream.R b/R/scream.R index ea8f7733..9b568fca 100644 --- a/R/scream.R +++ b/R/scream.R @@ -238,13 +238,13 @@ check_novel_levels <- function(x, ptype, column) { } warn_novel_levels <- function(levels, column) { - message <- glue( - "Novel levels found in column '{column}': {glue_quote_collapse(levels)}. ", - "The levels have been removed, and values have been coerced to 'NA'." - ) - - warn( - message, + n_levels <- length(levels) + cli::cli_warn( + c( + "{cli::qty(n_levels)}Novel level{?s} found in column {.field {column}}: {.val {levels}}.", + "i" = "The {cli::qty(n_levels)}level{?s} {?has/have} been removed, + and values have been coerced to {.val NA}." + ), class = "hardhat_warn_novel_levels", levels = levels, column = column diff --git a/tests/testthat/_snaps/forge-formula.md b/tests/testthat/_snaps/forge-formula.md index 01ab369e..a832436e 100644 --- a/tests/testthat/_snaps/forge-formula.md +++ b/tests/testthat/_snaps/forge-formula.md @@ -68,7 +68,8 @@ xx1 <- forge(new, x1$blueprint) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". --- @@ -76,7 +77,17 @@ xx2 <- forge(new, x2$blueprint) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". + +--- + + Code + xx3 <- forge(new_multiple, x3$blueprint) + Condition + Warning: + Novel levels found in column f: "e" and "f". + i The levels have been removed, and values have been coerced to "NA". # novel predictor levels can be ignored @@ -94,7 +105,8 @@ xx1 <- forge(new, x1$blueprint) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". --- @@ -102,7 +114,8 @@ xx2 <- forge(new, x2$blueprint) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". # novel levels are ignored correctly when the new column is a character @@ -120,7 +133,8 @@ xx1 <- forge(new, x1$blueprint, outcomes = TRUE) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". --- @@ -128,7 +142,8 @@ xx2 <- forge(new, x2$blueprint, outcomes = TRUE) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". # novel outcome levels are always caught, even if `allow_novel_levels = TRUE` @@ -136,7 +151,8 @@ xx1 <- forge(new, x1$blueprint, outcomes = TRUE) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". --- @@ -144,7 +160,8 @@ xx2 <- forge(new, x2$blueprint, outcomes = TRUE) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". # missing predictor levels are restored silently @@ -181,7 +198,8 @@ xx <- forge(new, x1$blueprint) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". # can be both missing levels and have new levels that get ignored @@ -199,5 +217,6 @@ out <- forge(df2, x$blueprint) Condition Warning: - Novel levels found in column 'x': 'd'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column x: "d". + i The level has been removed, and values have been coerced to "NA". diff --git a/tests/testthat/_snaps/forge-recipe.md b/tests/testthat/_snaps/forge-recipe.md index 5fb8b049..ed06b38f 100644 --- a/tests/testthat/_snaps/forge-recipe.md +++ b/tests/testthat/_snaps/forge-recipe.md @@ -36,7 +36,8 @@ xx1 <- forge(new, x1$blueprint) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". --- @@ -44,7 +45,8 @@ xx2 <- forge(new, x2$blueprint) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". Warning: ! There are new levels in `f`: NA. i Consider using step_unknown() (`?recipes::step_unknown()`) before `step_dummy()` to handle missing values. @@ -70,7 +72,8 @@ xx1 <- forge(new, x1$blueprint, outcomes = TRUE) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". --- @@ -78,7 +81,8 @@ xx2 <- forge(new, x2$blueprint, outcomes = TRUE) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". # `forge()` will error if required non standard roles are missing diff --git a/tests/testthat/_snaps/forge-xy.md b/tests/testthat/_snaps/forge-xy.md index e32e271f..99b50c98 100644 --- a/tests/testthat/_snaps/forge-xy.md +++ b/tests/testthat/_snaps/forge-xy.md @@ -80,7 +80,8 @@ xx <- forge(new, x$blueprint) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". # novel predictor levels can be ignored @@ -93,7 +94,8 @@ xx1 <- forge(new, x1$blueprint, outcomes = TRUE) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". --- @@ -101,5 +103,6 @@ xx2 <- forge(new, x2$blueprint, outcomes = TRUE) Condition Warning: - Novel levels found in column 'f': 'e'. The levels have been removed, and values have been coerced to 'NA'. + Novel level found in column f: "e". + i The level has been removed, and values have been coerced to "NA". diff --git a/tests/testthat/test-forge-formula.R b/tests/testthat/test-forge-formula.R index dfeebff0..e1cfdc6c 100644 --- a/tests/testthat/test-forge-formula.R +++ b/tests/testthat/test-forge-formula.R @@ -259,6 +259,14 @@ test_that("novel predictor levels are caught", { unname(xx2$predictors[5, 1]), NA_real_ ) + + new_multiple <- data.frame( + y = 1:6, + f = factor(letters[1:6]) + ) + x3 <- mold(y ~ f, dat) + + expect_snapshot(xx3 <- forge(new_multiple, x3$blueprint)) }) test_that("novel predictor levels can be ignored", { From 7fff0ce3b9564869f1867ab9e6c8b31df2412261 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Wed, 23 Oct 2024 16:16:17 +0100 Subject: [PATCH 15/36] Update `intercept.R` --- R/intercept.R | 12 ++++++------ tests/testthat/_snaps/intercept.md | 11 ++++++++++- tests/testthat/test-intercept.R | 6 ++++++ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/R/intercept.R b/R/intercept.R index 35e114ca..fa6d07e5 100644 --- a/R/intercept.R +++ b/R/intercept.R @@ -25,18 +25,18 @@ add_intercept_column <- function(data, name = "(Intercept)") { ok <- is.data.frame(data) || is.matrix(data) if (!ok) { - glubort( - "`data` must be a data.frame or matrix to add an intercept column, ", - "not a '{class1(data)}'." + cli::cli_abort( + "{.arg data} must be a data frame or matrix to add an intercept column, + not {.obj_type_friendly {data}}." ) } check_name(name) if (name %in% colnames(data)) { - warn(glue::glue( - "`data` already has a column named '{name}'. ", - "Returning `data` unchanged." + cli::cli_warn(c( + "{.arg data} already has a column named {.val {name}}.", + "i" = "Returning {.arg data} unchanged." )) return(data) diff --git a/tests/testthat/_snaps/intercept.md b/tests/testthat/_snaps/intercept.md index 9a1a8dcb..65eba53b 100644 --- a/tests/testthat/_snaps/intercept.md +++ b/tests/testthat/_snaps/intercept.md @@ -4,7 +4,8 @@ xx <- add_intercept_column(x) Condition Warning: - `data` already has a column named '(Intercept)'. Returning `data` unchanged. + `data` already has a column named "(Intercept)". + i Returning `data` unchanged. # name can only be a single character @@ -22,3 +23,11 @@ Error in `add_intercept_column()`: ! `name` must be a valid name, not the number 1. +# data has to be a data frame or matrix + + Code + add_intercept_column(1) + Condition + Error in `add_intercept_column()`: + ! `data` must be a data frame or matrix to add an intercept column, not a number. + diff --git a/tests/testthat/test-intercept.R b/tests/testthat/test-intercept.R index 79d42820..9bf8ebd5 100644 --- a/tests/testthat/test-intercept.R +++ b/tests/testthat/test-intercept.R @@ -37,3 +37,9 @@ test_that("name can only be a single character", { add_intercept_column(mtcars, name = 1) }) }) + +test_that("data has to be a data frame or matrix", { + expect_snapshot(error = TRUE, { + add_intercept_column(1) + }) +}) From 986368c74936359ab3f403533c038ac1f8a88583 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Thu, 24 Oct 2024 13:00:36 +0100 Subject: [PATCH 16/36] harmonize message for `.default()` methods again --- R/forge.R | 4 ++-- R/mold.R | 2 +- R/standardize.R | 2 +- tests/testthat/_snaps/forge-formula.md | 4 ++-- tests/testthat/_snaps/forge-xy.md | 6 +++--- tests/testthat/_snaps/forge.md | 2 +- tests/testthat/_snaps/levels.md | 2 +- tests/testthat/_snaps/mold.md | 2 +- tests/testthat/_snaps/standardize.md | 4 ++-- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/R/forge.R b/R/forge.R index 06545428..1785b83e 100644 --- a/R/forge.R +++ b/R/forge.R @@ -70,7 +70,7 @@ forge <- function(new_data, blueprint, ..., outcomes = FALSE) { #' @export forge.default <- function(new_data, blueprint, ..., outcomes = FALSE) { - cli::cli_abort("No {.fn forge} method provided for {.obj_type_friendly {new_data}} object.") + cli::cli_abort("No {.fn forge} method provided for {.obj_type_friendly {new_data}}.") } #' @export @@ -140,5 +140,5 @@ run_forge.default <- function(blueprint, new_data, ..., outcomes = FALSE) { - cli::cli_abort("No {.fn run_forge} method provided for {.obj_type_friendly {blueprint}} object.") + cli::cli_abort("No {.fn run_forge} method provided for {.obj_type_friendly {blueprint}}.") } diff --git a/R/mold.R b/R/mold.R index b34223c7..b770e145 100644 --- a/R/mold.R +++ b/R/mold.R @@ -175,5 +175,5 @@ run_mold <- function(blueprint, ...) { #' @export run_mold.default <- function(blueprint, ...) { - cli::cli_abort("No {.fn run_mold} method provided for {.obj_type_friendly {blueprint}} object.") + cli::cli_abort("No {.fn run_mold} method provided for {.obj_type_friendly {blueprint}}.") } diff --git a/R/standardize.R b/R/standardize.R index 6e63502b..6fe7da7f 100644 --- a/R/standardize.R +++ b/R/standardize.R @@ -39,7 +39,7 @@ standardize <- function(y) { #' @export standardize.default <- function(y) { - glubort("`y` is of unknown type '{class1(y)}'.") + cli::cli_abort("No {.fn standardize} method provided for {.obj_type_friendly {y}}.") } #' @export diff --git a/tests/testthat/_snaps/forge-formula.md b/tests/testthat/_snaps/forge-formula.md index a832436e..7aa76f60 100644 --- a/tests/testthat/_snaps/forge-formula.md +++ b/tests/testthat/_snaps/forge-formula.md @@ -20,7 +20,7 @@ forge("hi", x1$blueprint) Condition Error in `forge()`: - ! No `forge()` method provided for a string object. + ! No `forge()` method provided for a string. --- @@ -28,7 +28,7 @@ forge("hi", x2$blueprint) Condition Error in `forge()`: - ! No `forge()` method provided for a string object. + ! No `forge()` method provided for a string. # missing predictor columns fail appropriately diff --git a/tests/testthat/_snaps/forge-xy.md b/tests/testthat/_snaps/forge-xy.md index 99b50c98..5e611d7b 100644 --- a/tests/testthat/_snaps/forge-xy.md +++ b/tests/testthat/_snaps/forge-xy.md @@ -24,7 +24,7 @@ forge("hi", x1$blueprint) Condition Error in `forge()`: - ! No `forge()` method provided for a string object. + ! No `forge()` method provided for a string. --- @@ -32,7 +32,7 @@ forge("hi", x2$blueprint) Condition Error in `forge()`: - ! No `forge()` method provided for a string object. + ! No `forge()` method provided for a string. --- @@ -40,7 +40,7 @@ forge("hi", x3$blueprint) Condition Error in `forge()`: - ! No `forge()` method provided for a string object. + ! No `forge()` method provided for a string. # missing predictor columns fail appropriately diff --git a/tests/testthat/_snaps/forge.md b/tests/testthat/_snaps/forge.md index 86af191f..4e2f2e78 100644 --- a/tests/testthat/_snaps/forge.md +++ b/tests/testthat/_snaps/forge.md @@ -4,5 +4,5 @@ run_forge(1) Condition Error in `run_forge()`: - ! No `run_forge()` method provided for a number object. + ! No `run_forge()` method provided for a number. diff --git a/tests/testthat/_snaps/levels.md b/tests/testthat/_snaps/levels.md index e8207e5e..444eff85 100644 --- a/tests/testthat/_snaps/levels.md +++ b/tests/testthat/_snaps/levels.md @@ -4,5 +4,5 @@ get_outcome_levels("a") Condition Error in `standardize()`: - ! `y` is of unknown type 'character'. + ! No `standardize()` method provided for a string. diff --git a/tests/testthat/_snaps/mold.md b/tests/testthat/_snaps/mold.md index 94dc935c..1dc5ace4 100644 --- a/tests/testthat/_snaps/mold.md +++ b/tests/testthat/_snaps/mold.md @@ -4,5 +4,5 @@ run_mold(1) Condition Error in `run_mold()`: - ! No `run_mold()` method provided for a number object. + ! No `run_mold()` method provided for a number. diff --git a/tests/testthat/_snaps/standardize.md b/tests/testthat/_snaps/standardize.md index 23967906..a2dc43a2 100644 --- a/tests/testthat/_snaps/standardize.md +++ b/tests/testthat/_snaps/standardize.md @@ -52,7 +52,7 @@ standardize("hi") Condition Error in `standardize()`: - ! `y` is of unknown type 'character'. + ! No `standardize()` method provided for a string. --- @@ -60,5 +60,5 @@ standardize(Sys.time()) Condition Error in `standardize()`: - ! `y` is of unknown type 'POSIXct'. + ! No `standardize()` method provided for a object. From 008faa2bb04787f242d889b7af8e20db9b10be07 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Thu, 24 Oct 2024 13:03:29 +0100 Subject: [PATCH 17/36] Use standard checker instead of `glubort()` --- R/validation.R | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/R/validation.R b/R/validation.R index d1e89583..d207b6fd 100644 --- a/R/validation.R +++ b/R/validation.R @@ -504,9 +504,7 @@ validate_column_names <- function(data, original_names) { #' @rdname validate_column_names #' @export check_column_names <- function(data, original_names) { - if (!is.character(original_names)) { - glubort("`original_names` must be a character vector.") - } + check_character(original_names) new_names <- colnames(data) From 93a590d62c189410defff0ea16a50cd79075171b Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Thu, 24 Oct 2024 14:44:31 +0100 Subject: [PATCH 18/36] replace `glubort()` in `model-offset.R` --- R/model-offset.R | 6 +++--- tests/testthat/_snaps/model-offset.md | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/R/model-offset.R b/R/model-offset.R index f35c08b6..fdcd3918 100644 --- a/R/model-offset.R +++ b/R/model-offset.R @@ -63,9 +63,9 @@ model_offset <- function(terms, data) { if (!is.numeric(.offset_val)) { bad_col <- colnames(data)[.pos] - glubort( - "Column, '{bad_col}', is tagged as an offset, but is not numeric. ", - "All offsets must be numeric." + cli::cli_abort( + "Column {.field {bad_col}} is tagged as an offset and thus must be + numeric, not {.obj_type_friendly { .offset_val }}." ) } diff --git a/tests/testthat/_snaps/model-offset.md b/tests/testthat/_snaps/model-offset.md index 316ca181..7cc5769a 100644 --- a/tests/testthat/_snaps/model-offset.md +++ b/tests/testthat/_snaps/model-offset.md @@ -4,7 +4,7 @@ mold(~ Sepal.Width + offset(Species), iris) Condition Error in `model_offset()`: - ! Column, 'offset(Species)', is tagged as an offset, but is not numeric. All offsets must be numeric. + ! Column offset(Species) is tagged as an offset and thus must be numeric, not a object. # offset columns are stored as predictors From 3756c5d22f0f9a9cc480b5e890eac1527fecc6a2 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Thu, 24 Oct 2024 14:45:31 +0100 Subject: [PATCH 19/36] replace `glubort()` in `spurce.R` --- R/spruce.R | 6 +++--- tests/testthat/_snaps/spruce.md | 6 ++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/R/spruce.R b/R/spruce.R index 3ac85be8..cbac2bc9 100644 --- a/R/spruce.R +++ b/R/spruce.R @@ -66,9 +66,9 @@ spruce_prob <- function(pred_levels, prob_matrix) { n_col <- ncol(prob_matrix) if (n_levels != n_col) { - glubort( - "The number of levels ({n_levels}) must be - equal to the number of class probability columns ({n_col})." + cli::cli_abort( + "The number of levels ({n_levels}) must be equal to the number + of class probability columns ({n_col})." ) } diff --git a/tests/testthat/_snaps/spruce.md b/tests/testthat/_snaps/spruce.md index cb6cc552..7daa524f 100644 --- a/tests/testthat/_snaps/spruce.md +++ b/tests/testthat/_snaps/spruce.md @@ -60,8 +60,7 @@ spruce_prob(c("a", "b"), matrix(1, ncol = 3)) Condition Error in `spruce_prob()`: - ! The number of levels (2) must be - equal to the number of class probability columns (3). + ! The number of levels (2) must be equal to the number of class probability columns (3). --- @@ -69,8 +68,7 @@ spruce_prob(c("a"), matrix(1, ncol = 2)) Condition Error in `spruce_prob()`: - ! The number of levels (1) must be - equal to the number of class probability columns (2). + ! The number of levels (1) must be equal to the number of class probability columns (2). # spruce multiple helpers check input type From 62bddfd86b3f5aade204ad93da4691bc69394f6a Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Thu, 24 Oct 2024 14:46:50 +0100 Subject: [PATCH 20/36] replace `glubort()` in `standardize.R` --- R/standardize.R | 13 +++++++------ tests/testthat/_snaps/standardize.md | 12 +++++++++++- tests/testthat/test-standardize.R | 4 ++++ 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/R/standardize.R b/R/standardize.R index 6fe7da7f..a2ee1b44 100644 --- a/R/standardize.R +++ b/R/standardize.R @@ -80,7 +80,7 @@ standardize.array <- function(y) { } else if (dims(y) == 2) { standardize.matrix(y) } else { - glubort("3D+ arrays are not supported outcome types.") + cli::cli_abort("3D+ arrays are not supported outcome types.") } } @@ -101,11 +101,12 @@ validate_has_known_outcome_types <- function(y) { if (!all(known)) { not_known <- which(!known) not_known <- colnames(y)[not_known] - not_known <- glue_quote_collapse(not_known) - - glubort( - "Not all columns of `y` are known outcome types. ", - "These columns have unknown types: {not_known}." + cli::cli_abort( + c( + "Not all columns of {.arg y} are known outcome types.", + "i" = "{?This/These} column{?s} {?has/have} {?an/} unknown type{?s}: + {.val {not_known}}." + ) ) } diff --git a/tests/testthat/_snaps/standardize.md b/tests/testthat/_snaps/standardize.md index a2dc43a2..9414fef4 100644 --- a/tests/testthat/_snaps/standardize.md +++ b/tests/testthat/_snaps/standardize.md @@ -44,7 +44,17 @@ standardize(bad2) Condition Error in `validate_has_known_outcome_types()`: - ! Not all columns of `y` are known outcome types. These columns have unknown types: 'x'. + ! Not all columns of `y` are known outcome types. + i This column has an unknown type: "x". + +--- + + Code + standardize(bad3) + Condition + Error in `validate_has_known_outcome_types()`: + ! Not all columns of `y` are known outcome types. + i These columns have unknown types: "x" and "y". # standardize - unknown diff --git a/tests/testthat/test-standardize.R b/tests/testthat/test-standardize.R index bb6aff38..e85469a2 100644 --- a/tests/testthat/test-standardize.R +++ b/tests/testthat/test-standardize.R @@ -73,6 +73,10 @@ test_that("standardize - data.frame", { expect_snapshot(error = TRUE, standardize(bad2)) + bad3 <- data.frame(x = "a", y = "b", stringsAsFactors = FALSE) + + expect_snapshot(error = TRUE, standardize(bad3)) + good <- bad colnames(good) <- c("a", "b") From 13449ebc0fd192ef96ad3071cd7bac5b7c2e771f Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Thu, 24 Oct 2024 14:48:07 +0100 Subject: [PATCH 21/36] Replace `glubort()` in `util.R` --- R/util.R | 11 +++++++---- tests/testthat/_snaps/mold-formula.md | 10 +++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/R/util.R b/R/util.R index 29328d0a..db7ca8ee 100644 --- a/R/util.R +++ b/R/util.R @@ -45,8 +45,10 @@ get_all_predictors <- function(formula, data) { extra_predictors <- setdiff(predictors, names(data)) if (length(extra_predictors) > 0) { - extra_predictors <- glue_quote_collapse(extra_predictors) - glubort("The following predictors were not found in `data`: {extra_predictors}.") + cli::cli_abort( + "The following predictor{?s} {?was/were} not found in + {.arg data}: {.val {extra_predictors}}." + ) } predictors @@ -69,8 +71,9 @@ get_all_outcomes <- function(formula, data) { extra_outcomes <- setdiff(outcomes, names(data)) if (length(extra_outcomes) > 0) { - extra_outcomes <- glue_quote_collapse(extra_outcomes) - glubort("The following outcomes were not found in `data`: {extra_outcomes}.") + cli::cli_abort( + "The following outcome{?s} {?was/were} not found in {.arg data}: + {.val {extra_outcomes}}.") } outcomes diff --git a/tests/testthat/_snaps/mold-formula.md b/tests/testthat/_snaps/mold-formula.md index aa490dfb..0ae47b6d 100644 --- a/tests/testthat/_snaps/mold-formula.md +++ b/tests/testthat/_snaps/mold-formula.md @@ -127,7 +127,7 @@ mold(fac_1 ~ y + z, example_train) Condition Error in `get_all_predictors()`: - ! The following predictors were not found in `data`: 'y', 'z'. + ! The following predictors were not found in `data`: "y" and "z". --- @@ -135,7 +135,7 @@ mold(fac_1 ~ y + z, example_train, blueprint = bp) Condition Error in `get_all_predictors()`: - ! The following predictors were not found in `data`: 'y', 'z'. + ! The following predictors were not found in `data`: "y" and "z". --- @@ -143,7 +143,7 @@ mold(y + z ~ fac_1, example_train) Condition Error in `get_all_outcomes()`: - ! The following outcomes were not found in `data`: 'y', 'z'. + ! The following outcomes were not found in `data`: "y" and "z". --- @@ -151,7 +151,7 @@ mold(y + z ~ fac_1, example_train, blueprint = bp) Condition Error in `get_all_outcomes()`: - ! The following outcomes were not found in `data`: 'y', 'z'. + ! The following outcomes were not found in `data`: "y" and "z". # global environment variables cannot be used @@ -160,7 +160,7 @@ mold(fac_1 ~ y, example_train) Condition Error in `get_all_predictors()`: - ! The following predictors were not found in `data`: 'y'. + ! The following predictor was not found in `data`: "y". # cannot manually remove intercept in the formula itself From e0822b71c457e0f8df5b07b871e856f6fb823abd Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Thu, 24 Oct 2024 15:16:17 +0100 Subject: [PATCH 22/36] Replace `glubort()` in `validation.R` --- R/validation.R | 117 ++++++++++++++-------------- tests/testthat/_snaps/forge-xy.md | 12 +-- tests/testthat/_snaps/validation.md | 59 ++++++++------ 3 files changed, 100 insertions(+), 88 deletions(-) diff --git a/R/validation.R b/R/validation.R index d207b6fd..685e5963 100644 --- a/R/validation.R +++ b/R/validation.R @@ -39,7 +39,7 @@ validate_outcomes_are_univariate <- function(outcomes) { check <- check_outcomes_are_univariate(outcomes) if (!check$ok) { - glubort( + cli::cli_abort( "The outcome must be univariate, but {check$n_cols} columns were found." ) } @@ -111,16 +111,16 @@ validate_outcomes_are_numeric <- function(outcomes) { check <- check_outcomes_are_numeric(outcomes) if (!check$ok) { - bad_cols <- glue::single_quote(names(check$bad_classes)) - bad_printable_classes <- map(check$bad_classes, glue_quote_collapse) - bad_msg <- glue::glue("{bad_cols}: {bad_printable_classes}") - bad_msg <- glue::glue_collapse(bad_msg, sep = "\n") - - glubort( - "All outcomes must be numeric, but the following are not:", - "\n", - "{bad_msg}" - ) + bad_col <- map(names(check$bad_classes), ~ cli::format_inline("{.val {.x}}")) + bad_class <- map(check$bad_classes, ~ cli::format_inline("{.cls {.x}}")) + + bad_msg <- glue::glue("{bad_col}: {bad_class}") + + cli::cli_abort(c( + "All outcomes must be numeric.", + "i" = "{cli::qty(length(bad_class))}The following {?is/are} not:", + bad_msg + )) } invisible(outcomes) @@ -190,16 +190,16 @@ validate_outcomes_are_factors <- function(outcomes) { check <- check_outcomes_are_factors(outcomes) if (!check$ok) { - bad_cols <- glue::single_quote(names(check$bad_classes)) - bad_printable_classes <- map(check$bad_classes, glue_quote_collapse) - bad_msg <- glue::glue("{bad_cols}: {bad_printable_classes}") - bad_msg <- glue::glue_collapse(bad_msg, sep = "\n") - - glubort( - "All outcomes must be factors, but the following are not:", - "\n", - "{bad_msg}" - ) + bad_col <- map(names(check$bad_classes), ~ cli::format_inline("{.val {.x}}")) + bad_class <- map(check$bad_classes, ~ cli::format_inline("{.cls {.x}}")) + + bad_msg <- glue::glue("{bad_col}: {bad_class}") + + cli::cli_abort(c( + "All outcomes must be factors.", + "i" = "{cli::qty(length(bad_class))}The following {?is/are} not:", + bad_msg + )) } invisible(outcomes) @@ -274,16 +274,16 @@ validate_outcomes_are_binary <- function(outcomes) { check <- check_outcomes_are_binary(outcomes) if (!check$ok) { - bad_cols <- glue::single_quote(check$bad_cols) - bad_msg <- glue::glue("{bad_cols}: {check$num_levels}") - bad_msg <- glue::glue_collapse(bad_msg, sep = "\n") - - glubort( - "The outcome must be binary, ", - "but the following number of levels were found:", - "\n", - "{bad_msg}" - ) + bad_col <- map(check$bad_cols, ~ cli::format_inline("{.val {.x}}")) + + bad_msg <- glue::glue("{bad_col}: {check$num_levels}") + + cli::cli_abort(c( + "The outcome must be binary.", + "i" = "{cli::qty(length(bad_col))}The following number of levels + {?was/were} found:", + bad_msg + )) } invisible(outcomes) @@ -366,16 +366,16 @@ validate_predictors_are_numeric <- function(predictors) { check <- check_predictors_are_numeric(predictors) if (!check$ok) { - bad_cols <- glue::single_quote(names(check$bad_classes)) - bad_printable_classes <- map(check$bad_classes, glue_quote_collapse) - bad_msg <- glue::glue("{bad_cols}: {bad_printable_classes}") - bad_msg <- glue::glue_collapse(bad_msg, sep = "\n") - - glubort( - "All predictors must be numeric, but the following are not:", - "\n", - "{bad_msg}" - ) + bad_col <- map(names(check$bad_classes), ~ cli::format_inline("{.val {.x}}")) + bad_class <- map(check$bad_classes, ~ cli::format_inline("{.cls {.x}}")) + + bad_msg <- glue::glue("{bad_col}: {bad_class}") + + cli::cli_abort(c( + "All predictors must be numeric.", + "i" = "{cli::qty(length(bad_class))}The following {?is/are} not:", + bad_msg + )) } invisible(predictors) @@ -525,15 +525,14 @@ validate_missing_name_isnt_.outcome <- function(missing_names) { not_ok <- ".outcome" %in% missing_names if (not_ok) { - missing_names <- glue_quote_collapse(missing_names) - - glubort( - "The following required columns are missing: {missing_names}. - - (This indicates that `mold()` was called with a vector for `y`. ", - "When this is the case, and the outcome columns are requested ", - "in `forge()`, `new_data` must include a column with the automatically ", - "generated name, '.outcome', containing the outcome.)" + cli::cli_abort( + c( + "The following required columns are missing: {.val {missing_names}}.", + "i" = "This indicates that {.fn mold} was called with a vector for {.arg y}.", + "i" = "When this is the case, and the outcome columns are requested in + {.fn forge}, {.arg new_data} must include a column with the + automatically generated name, {.code .outcome}, containing the outcome." + ) ) } @@ -609,9 +608,9 @@ validate_prediction_size <- function(pred, new_data) { check <- check_prediction_size(pred, new_data) if (!check$ok) { - glubort( - "The size of `new_data` ({check$size_new_data}) must match the ", - "size of `pred` ({check$size_pred})." + cli::cli_abort( + "The size of {.arg new_data} ({check$size_new_data}) must match the + size of {.arg pred} ({check$size_pred})." ) } @@ -686,11 +685,13 @@ validate_no_formula_duplication <- function(formula, original = FALSE) { check <- check_no_formula_duplication(formula, original) if (!check$ok) { - duplicates <- glue_quote_collapse(check$duplicates) - - glubort( - "The following terms are duplicated on the left and right hand side ", - "of the `formula`: {duplicates}." + cli::cli_abort( + c( + "Terms must not be duplicated on the left- and right-hand side + of the {.arg formula}.", + "i" = "The following duplicated term{?s} {?was/were} found: + {.val {check$duplicates}}" + ) ) } diff --git a/tests/testthat/_snaps/forge-xy.md b/tests/testthat/_snaps/forge-xy.md index 5e611d7b..d1625a90 100644 --- a/tests/testthat/_snaps/forge-xy.md +++ b/tests/testthat/_snaps/forge-xy.md @@ -4,9 +4,9 @@ forge(iris, x1$blueprint, outcomes = TRUE) Condition Error in `validate_missing_name_isnt_.outcome()`: - ! The following required columns are missing: '.outcome'. - - (This indicates that `mold()` was called with a vector for `y`. When this is the case, and the outcome columns are requested in `forge()`, `new_data` must include a column with the automatically generated name, '.outcome', containing the outcome.) + ! The following required columns are missing: ".outcome". + i This indicates that `mold()` was called with a vector for `y`. + i When this is the case, and the outcome columns are requested in `forge()`, `new_data` must include a column with the automatically generated name, `.outcome`, containing the outcome. --- @@ -14,9 +14,9 @@ forge(iris, x2$blueprint, outcomes = TRUE) Condition Error in `validate_missing_name_isnt_.outcome()`: - ! The following required columns are missing: '.outcome'. - - (This indicates that `mold()` was called with a vector for `y`. When this is the case, and the outcome columns are requested in `forge()`, `new_data` must include a column with the automatically generated name, '.outcome', containing the outcome.) + ! The following required columns are missing: ".outcome". + i This indicates that `mold()` was called with a vector for `y`. + i When this is the case, and the outcome columns are requested in `forge()`, `new_data` must include a column with the automatically generated name, `.outcome`, containing the outcome. # new_data can only be a data frame / matrix diff --git a/tests/testthat/_snaps/validation.md b/tests/testthat/_snaps/validation.md index 4c397ab9..afb2fb5c 100644 --- a/tests/testthat/_snaps/validation.md +++ b/tests/testthat/_snaps/validation.md @@ -12,8 +12,9 @@ validate_outcomes_are_numeric(iris) Condition Error in `validate_outcomes_are_numeric()`: - ! All outcomes must be numeric, but the following are not: - 'Species': 'factor' + ! All outcomes must be numeric. + i The following is not: + "Species": --- @@ -21,9 +22,10 @@ validate_outcomes_are_numeric(x) Condition Error in `validate_outcomes_are_numeric()`: - ! All outcomes must be numeric, but the following are not: - 'x': 'POSIXct', 'POSIXt' - 'y': 'factor' + ! All outcomes must be numeric. + i The following are not: + "x": + "y": # validate_no_formula_duplication() @@ -31,7 +33,8 @@ validate_no_formula_duplication(y ~ y) Condition Error in `validate_no_formula_duplication()`: - ! The following terms are duplicated on the left and right hand side of the `formula`: 'y'. + ! Terms must not be duplicated on the left- and right-hand side of the `formula`. + i The following duplicated term was found: "y" --- @@ -39,7 +42,8 @@ validate_no_formula_duplication(y ~ log(y), original = TRUE) Condition Error in `validate_no_formula_duplication()`: - ! The following terms are duplicated on the left and right hand side of the `formula`: 'y'. + ! Terms must not be duplicated on the left- and right-hand side of the `formula`. + i The following duplicated term was found: "y" --- @@ -47,7 +51,8 @@ validate_no_formula_duplication(y + x ~ y + x) Condition Error in `validate_no_formula_duplication()`: - ! The following terms are duplicated on the left and right hand side of the `formula`: 'y', 'x'. + ! Terms must not be duplicated on the left- and right-hand side of the `formula`. + i The following duplicated terms were found: "y" and "x" --- @@ -55,7 +60,8 @@ validate_no_formula_duplication(y ~ . + y) Condition Error in `validate_no_formula_duplication()`: - ! The following terms are duplicated on the left and right hand side of the `formula`: 'y'. + ! Terms must not be duplicated on the left- and right-hand side of the `formula`. + i The following duplicated term was found: "y" --- @@ -63,7 +69,8 @@ validate_no_formula_duplication(y ~ offset(y), original = TRUE) Condition Error in `validate_no_formula_duplication()`: - ! The following terms are duplicated on the left and right hand side of the `formula`: 'y'. + ! Terms must not be duplicated on the left- and right-hand side of the `formula`. + i The following duplicated term was found: "y" # validate_outcomes_are_factors() @@ -71,9 +78,10 @@ validate_outcomes_are_factors(x) Condition Error in `validate_outcomes_are_factors()`: - ! All outcomes must be factors, but the following are not: - 'x': 'POSIXct', 'POSIXt' - 'y': 'character' + ! All outcomes must be factors. + i The following are not: + "x": + "y": # validate_outcomes_are_binary() @@ -81,12 +89,13 @@ validate_outcomes_are_binary(iris) Condition Error in `validate_outcomes_are_binary()`: - ! The outcome must be binary, but the following number of levels were found: - 'Sepal.Length': 0 - 'Sepal.Width': 0 - 'Petal.Length': 0 - 'Petal.Width': 0 - 'Species': 3 + ! The outcome must be binary. + i The following number of levels were found: + "Sepal.Length": 0 + "Sepal.Width": 0 + "Petal.Length": 0 + "Petal.Width": 0 + "Species": 3 # validate_predictors_are_numeric() @@ -94,8 +103,9 @@ validate_predictors_are_numeric(iris) Condition Error in `validate_predictors_are_numeric()`: - ! All predictors must be numeric, but the following are not: - 'Species': 'factor' + ! All predictors must be numeric. + i The following is not: + "Species": --- @@ -103,9 +113,10 @@ validate_predictors_are_numeric(x) Condition Error in `validate_predictors_are_numeric()`: - ! All predictors must be numeric, but the following are not: - 'x': 'POSIXct', 'POSIXt' - 'y': 'factor' + ! All predictors must be numeric. + i The following are not: + "x": + "y": # validate_prediction_size() From 060be048a10661d5ad99b588a71d489178a7935b Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Thu, 24 Oct 2024 15:19:04 +0100 Subject: [PATCH 23/36] Remove now unused helpers --- R/util.R | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/R/util.R b/R/util.R index db7ca8ee..256e2b8d 100644 --- a/R/util.R +++ b/R/util.R @@ -1,11 +1,3 @@ -glubort <- function(..., .sep = "", .envir = caller_env(), .call = .envir) { - abort(glue(..., .sep = .sep, .envir = .envir), call = .call) -} - -glue_quote_collapse <- function(x) { - glue::glue_collapse(glue::single_quote(x), sep = ", ") -} - simplify_terms <- function(x) { # This is like stats:::terms.default @@ -153,10 +145,6 @@ has_unique_column_names <- function(x) { !anyDuplicated(nms) } -class1 <- function(x) { - class(x)[1] -} - # ------------------------------------------------------------------------------ check_data_frame_or_matrix <- function(x, From 4357adf1050828c19f92f7c2b7e7fa8b951d8f2c Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Thu, 24 Oct 2024 16:14:49 +0100 Subject: [PATCH 24/36] use cli for `print()` methods for blueprints moved extra line for indicators into the parent method so that it's part of the same cli paragraph --- NAMESPACE | 1 - R/print.R | 24 ++++++------ tests/testthat/_snaps/print.md | 68 +++++++++++++++++----------------- 3 files changed, 45 insertions(+), 48 deletions(-) diff --git a/NAMESPACE b/NAMESPACE index e3d2995a..9d9739dd 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -16,7 +16,6 @@ S3method(mold,formula) S3method(mold,matrix) S3method(mold,recipe) S3method(obj_print_footer,quantile_pred) -S3method(print,formula_blueprint) S3method(print,hardhat_blueprint) S3method(print,hardhat_model) S3method(refresh_blueprint,default_formula_blueprint) diff --git a/R/print.R b/R/print.R index d0a09f2e..1974b19a 100644 --- a/R/print.R +++ b/R/print.R @@ -9,20 +9,18 @@ format.formula_blueprint <- function(x, ...) "Formula" #' @export print.hardhat_blueprint <- function(x, ...) { - cat_line("{format(x)} blueprint:") - cat_line("\n") - cat_line("# Predictors: {n_blueprint_predictors(x)}") - cat_line(" # Outcomes: {n_blueprint_outcomes(x)}") - cat_line(" Intercept: {x$intercept}") - cat_line("Novel Levels: {x$allow_novel_levels}") - cat_line(" Composition: {x$composition}") - invisible(x) -} + cli::cli_text("{format(x)} blueprint:") -#' @export -print.formula_blueprint <- function(x, ...) { - NextMethod() - cat_line(" Indicators: {x$indicators}") + cli::cli_par() + cli::cli_text("# Predictors: {n_blueprint_predictors(x)}") + cli::cli_text("# Outcomes: {n_blueprint_outcomes(x)}") + cli::cli_text("Intercept: {x$intercept}") + cli::cli_text("Novel Levels: {x$allow_novel_levels}") + cli::cli_text("Composition: {x$composition}") + if (inherits(x, "formula_blueprint")) { + cli::cli_text("Indicators: {x$indicators}") + } + cli::cli_end() invisible(x) } diff --git a/tests/testthat/_snaps/print.md b/tests/testthat/_snaps/print.md index 8af41660..aeedddc3 100644 --- a/tests/testthat/_snaps/print.md +++ b/tests/testthat/_snaps/print.md @@ -2,50 +2,50 @@ Code mold(Species ~ Sepal.Length, iris)$blueprint - Output - Formula blueprint: - - # Predictors: 1 - # Outcomes: 1 - Intercept: FALSE - Novel Levels: FALSE - Composition: tibble - Indicators: traditional + Message + Formula blueprint: + # Predictors: 1 + # Outcomes: 1 + Intercept: FALSE + Novel Levels: FALSE + Composition: tibble + Indicators: traditional + Code mold(~Sepal.Length, iris)$blueprint - Output - Formula blueprint: - - # Predictors: 1 - # Outcomes: 0 - Intercept: FALSE - Novel Levels: FALSE - Composition: tibble - Indicators: traditional + Message + Formula blueprint: + # Predictors: 1 + # Outcomes: 0 + Intercept: FALSE + Novel Levels: FALSE + Composition: tibble + Indicators: traditional + # print - default Code mold(iris[, c("Sepal.Length"), drop = FALSE], iris$Species)$blueprint - Output - XY blueprint: - - # Predictors: 1 - # Outcomes: 1 - Intercept: FALSE - Novel Levels: FALSE - Composition: tibble + Message + XY blueprint: + # Predictors: 1 + # Outcomes: 1 + Intercept: FALSE + Novel Levels: FALSE + Composition: tibble + # print - recipe Code mold(recipes::recipe(Species ~ Sepal.Length, iris), iris)$blueprint - Output - Recipe blueprint: - - # Predictors: 1 - # Outcomes: 1 - Intercept: FALSE - Novel Levels: FALSE - Composition: tibble + Message + Recipe blueprint: + # Predictors: 1 + # Outcomes: 1 + Intercept: FALSE + Novel Levels: FALSE + Composition: tibble + From e6ca5d1a41b018c1ba23483319b75ef6cae5a265 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Thu, 24 Oct 2024 16:17:16 +0100 Subject: [PATCH 25/36] remove now unused version of `cat_line()` the remaining version (previously overwritten) in `constructor.R` causes the snapshot change --- R/print.R | 5 ----- tests/testthat/_snaps/constructor.md | 6 +++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/R/print.R b/R/print.R index 1974b19a..2a479d85 100644 --- a/R/print.R +++ b/R/print.R @@ -24,11 +24,6 @@ print.hardhat_blueprint <- function(x, ...) { invisible(x) } -cat_line <- function(..., .envir = parent.frame()) { - lines <- paste(glue(..., .envir = .envir), "\n") - cat(lines, sep = "") -} - n_blueprint_predictors <- function(x) { ncol(x$ptypes$predictors) %||% 0L } diff --git a/tests/testthat/_snaps/constructor.md b/tests/testthat/_snaps/constructor.md index eb98eff3..4707063d 100644 --- a/tests/testthat/_snaps/constructor.md +++ b/tests/testthat/_snaps/constructor.md @@ -3,17 +3,17 @@ Code new_model() Output - + named list() Code new_model(class = "custom_class") Output - + named list() Code new_model(x = 4, y = "hi", class = "custom_class") Output - + $x [1] 4 From 45886c34e955f4ae289b4c1d2f342db5920a374f Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Fri, 25 Oct 2024 14:43:43 +0100 Subject: [PATCH 26/36] use custom check function --- R/util.R | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/R/util.R b/R/util.R index 256e2b8d..76ee3b4d 100644 --- a/R/util.R +++ b/R/util.R @@ -3,11 +3,7 @@ simplify_terms <- function(x) { # This is like stats:::terms.default # but doesn't look at x$terms. - is_terms <- inherits(x, "terms") - - if (!is_terms) { - cli::cli_abort("{.arg x} must be a {.cls terms} object.") - } + check_terms(x) # It removes the environment # (which could be large) From 685aaa562e1893a2fe450b72b43c6ba31422c55e Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Fri, 25 Oct 2024 14:44:10 +0100 Subject: [PATCH 27/36] follow pattern in the other checks on case weights --- R/case-weights.R | 2 +- tests/testthat/_snaps/case-weights.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/case-weights.R b/R/case-weights.R index a631d843..3fb80649 100644 --- a/R/case-weights.R +++ b/R/case-weights.R @@ -265,7 +265,7 @@ vec_ptype_abbr.hardhat_frequency_weights <- function(x, ...) { #' new_case_weights(1:5, class = "my_weights") new_case_weights <- function(x, ..., class) { if (!is.integer(x) && !is.double(x)) { - cli::cli_abort("{.arg x} must be {.cls integer} or {.cls double} vector.") + cli::cli_abort("{.arg x} must be an integer or double vector.") } new_vctr( diff --git a/tests/testthat/_snaps/case-weights.md b/tests/testthat/_snaps/case-weights.md index 63b23296..102fe87b 100644 --- a/tests/testthat/_snaps/case-weights.md +++ b/tests/testthat/_snaps/case-weights.md @@ -61,5 +61,5 @@ new_case_weights("x", class = "subclass") Condition Error in `new_case_weights()`: - ! `x` must be or vector. + ! `x` must be an integer or double vector. From 995e404d6922bab327e160153148a587a6dc2de8 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Fri, 25 Oct 2024 15:36:27 +0100 Subject: [PATCH 28/36] make helper for styling info on "bad classes" --- R/validation.R | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/R/validation.R b/R/validation.R index 685e5963..04dafeae 100644 --- a/R/validation.R +++ b/R/validation.R @@ -111,14 +111,11 @@ validate_outcomes_are_numeric <- function(outcomes) { check <- check_outcomes_are_numeric(outcomes) if (!check$ok) { - bad_col <- map(names(check$bad_classes), ~ cli::format_inline("{.val {.x}}")) - bad_class <- map(check$bad_classes, ~ cli::format_inline("{.cls {.x}}")) - - bad_msg <- glue::glue("{bad_col}: {bad_class}") + bad_msg <- style_bad_classes(check$bad_classes) cli::cli_abort(c( "All outcomes must be numeric.", - "i" = "{cli::qty(length(bad_class))}The following {?is/are} not:", + "i" = "{cli::qty(length(check$bad_classes))}The following {?is/are} not:", bad_msg )) } @@ -126,6 +123,13 @@ validate_outcomes_are_numeric <- function(outcomes) { invisible(outcomes) } +style_bad_classes <- function(bad_classes){ + bad_col <- map(names(bad_classes), ~ cli::format_inline("{.val {.x}}")) + bad_class <- map(bad_classes, ~ cli::format_inline("{.cls {.x}}")) + + glue::glue("{bad_col}: {bad_class}") +} + #' @rdname validate_outcomes_are_numeric #' @export check_outcomes_are_numeric <- function(outcomes) { @@ -190,14 +194,11 @@ validate_outcomes_are_factors <- function(outcomes) { check <- check_outcomes_are_factors(outcomes) if (!check$ok) { - bad_col <- map(names(check$bad_classes), ~ cli::format_inline("{.val {.x}}")) - bad_class <- map(check$bad_classes, ~ cli::format_inline("{.cls {.x}}")) - - bad_msg <- glue::glue("{bad_col}: {bad_class}") + bad_msg <- style_bad_classes(check$bad_classes) cli::cli_abort(c( "All outcomes must be factors.", - "i" = "{cli::qty(length(bad_class))}The following {?is/are} not:", + "i" = "{cli::qty(length(check$bad_classes))}The following {?is/are} not:", bad_msg )) } @@ -366,14 +367,11 @@ validate_predictors_are_numeric <- function(predictors) { check <- check_predictors_are_numeric(predictors) if (!check$ok) { - bad_col <- map(names(check$bad_classes), ~ cli::format_inline("{.val {.x}}")) - bad_class <- map(check$bad_classes, ~ cli::format_inline("{.cls {.x}}")) - - bad_msg <- glue::glue("{bad_col}: {bad_class}") + bad_msg <- style_bad_classes(check$bad_classes) cli::cli_abort(c( "All predictors must be numeric.", - "i" = "{cli::qty(length(bad_class))}The following {?is/are} not:", + "i" = "{cli::qty(length(check$bad_classes))}The following {?is/are} not:", bad_msg )) } From cb5335e29c0934e8cafff66e117fbbd0baf829f8 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Fri, 25 Oct 2024 16:01:37 +0100 Subject: [PATCH 29/36] Style column names consistently with `{.val}` --- R/blueprint-formula-default.R | 4 +-- R/model-offset.R | 2 +- R/recompose.R | 2 +- R/scream.R | 2 +- R/validation.R | 4 ++- tests/testthat/_snaps/forge-formula.md | 34 +++++++++++++------------- tests/testthat/_snaps/forge-recipe.md | 24 +++++++++--------- tests/testthat/_snaps/forge-xy.md | 14 +++++------ tests/testthat/_snaps/model-offset.md | 4 +-- 9 files changed, 46 insertions(+), 44 deletions(-) diff --git a/R/blueprint-formula-default.R b/R/blueprint-formula-default.R index f20335ff..b91e0fc1 100644 --- a/R/blueprint-formula-default.R +++ b/R/blueprint-formula-default.R @@ -970,7 +970,7 @@ expr_check_no_factorish_in_functions <- function(expr, "Functions involving factors or characters have been detected on the ", "RHS of `formula`. These are not allowed when `indicators = \"none\"`." ), - i = "Functions involving factors were detected for {.str {name}} in {.arg {expr}}." + i = "Functions involving factors were detected for {.val {name}} in {.arg {expr}}." ) cli::cli_abort(message, call = error_call) @@ -1035,7 +1035,7 @@ expr_check_no_factorish_in_interaction_term <- function(expr, "Interaction terms involving factors or characters have been detected on the ", "RHS of `formula`. These are not allowed when `indicators = \"none\"`." ), - i = "Interactions terms involving factors were detected for {.str {name}} in {.arg {expr}}." + i = "Interactions terms involving factors were detected for {.val {name}} in {.arg {expr}}." ) cli::cli_abort(message, call = error_call) diff --git a/R/model-offset.R b/R/model-offset.R index fdcd3918..7cfcabde 100644 --- a/R/model-offset.R +++ b/R/model-offset.R @@ -64,7 +64,7 @@ model_offset <- function(terms, data) { bad_col <- colnames(data)[.pos] cli::cli_abort( - "Column {.field {bad_col}} is tagged as an offset and thus must be + "Column {.val {bad_col}} is tagged as an offset and thus must be numeric, not {.obj_type_friendly { .offset_val }}." ) } diff --git a/R/recompose.R b/R/recompose.R index 51e6a8eb..9592824b 100644 --- a/R/recompose.R +++ b/R/recompose.R @@ -77,7 +77,7 @@ coerce_to_matrix <- function(data, error_call = caller_env()) { message <- c( "{.arg data} must only contain numeric columns.", - i = "These columns aren't numeric: {.str {loc}}." + i = "These columns aren't numeric: {.val {loc}}." ) cli::cli_abort(message, call = error_call) diff --git a/R/scream.R b/R/scream.R index 9b568fca..13196886 100644 --- a/R/scream.R +++ b/R/scream.R @@ -241,7 +241,7 @@ warn_novel_levels <- function(levels, column) { n_levels <- length(levels) cli::cli_warn( c( - "{cli::qty(n_levels)}Novel level{?s} found in column {.field {column}}: {.val {levels}}.", + "{cli::qty(n_levels)}Novel level{?s} found in column {.val {column}}: {.val {levels}}.", "i" = "The {cli::qty(n_levels)}level{?s} {?has/have} been removed, and values have been coerced to {.val NA}." ), diff --git a/R/validation.R b/R/validation.R index 04dafeae..1b5be255 100644 --- a/R/validation.R +++ b/R/validation.R @@ -493,7 +493,9 @@ validate_column_names <- function(data, original_names) { if (!check$ok) { validate_missing_name_isnt_.outcome(check$missing_names) - cli::cli_abort("The required column{?s} {.arg {check$missing_names}} {?is/are} missing.") + cli::cli_abort( + "The required column{?s} {.val {check$missing_names}} {?is/are} missing." + ) } invisible(data) diff --git a/tests/testthat/_snaps/forge-formula.md b/tests/testthat/_snaps/forge-formula.md index 7aa76f60..fa622805 100644 --- a/tests/testthat/_snaps/forge-formula.md +++ b/tests/testthat/_snaps/forge-formula.md @@ -4,7 +4,7 @@ forge(example_train2, x1$blueprint, outcomes = TRUE) Condition Error in `validate_column_names()`: - ! The required column `fac_1` is missing. + ! The required column "fac_1" is missing. --- @@ -12,7 +12,7 @@ forge(example_train2, x2$blueprint, outcomes = TRUE) Condition Error in `validate_column_names()`: - ! The required column `fac_1` is missing. + ! The required column "fac_1" is missing. # new_data can only be a data frame / matrix @@ -36,7 +36,7 @@ forge(example_train[, 1, drop = FALSE], x1$blueprint) Condition Error in `validate_column_names()`: - ! The required column `num_2` is missing. + ! The required column "num_2" is missing. --- @@ -44,7 +44,7 @@ forge(example_train[, 1, drop = FALSE], x2$blueprint) Condition Error in `validate_column_names()`: - ! The required column `num_2` is missing. + ! The required column "num_2" is missing. --- @@ -52,7 +52,7 @@ forge(example_train[, 3, drop = FALSE], x1$blueprint) Condition Error in `validate_column_names()`: - ! The required columns `num_1` and `num_2` are missing. + ! The required columns "num_1" and "num_2" are missing. --- @@ -60,7 +60,7 @@ forge(example_train[, 3, drop = FALSE], x2$blueprint) Condition Error in `validate_column_names()`: - ! The required columns `num_1` and `num_2` are missing. + ! The required columns "num_1" and "num_2" are missing. # novel predictor levels are caught @@ -68,7 +68,7 @@ xx1 <- forge(new, x1$blueprint) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". --- @@ -77,7 +77,7 @@ xx2 <- forge(new, x2$blueprint) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". --- @@ -86,7 +86,7 @@ xx3 <- forge(new_multiple, x3$blueprint) Condition Warning: - Novel levels found in column f: "e" and "f". + Novel levels found in column "f": "e" and "f". i The levels have been removed, and values have been coerced to "NA". # novel predictor levels can be ignored @@ -105,7 +105,7 @@ xx1 <- forge(new, x1$blueprint) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". --- @@ -114,7 +114,7 @@ xx2 <- forge(new, x2$blueprint) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". # novel levels are ignored correctly when the new column is a character @@ -133,7 +133,7 @@ xx1 <- forge(new, x1$blueprint, outcomes = TRUE) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". --- @@ -142,7 +142,7 @@ xx2 <- forge(new, x2$blueprint, outcomes = TRUE) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". # novel outcome levels are always caught, even if `allow_novel_levels = TRUE` @@ -151,7 +151,7 @@ xx1 <- forge(new, x1$blueprint, outcomes = TRUE) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". --- @@ -160,7 +160,7 @@ xx2 <- forge(new, x2$blueprint, outcomes = TRUE) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". # missing predictor levels are restored silently @@ -198,7 +198,7 @@ xx <- forge(new, x1$blueprint) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". # can be both missing levels and have new levels that get ignored @@ -217,6 +217,6 @@ out <- forge(df2, x$blueprint) Condition Warning: - Novel level found in column x: "d". + Novel level found in column "x": "d". i The level has been removed, and values have been coerced to "NA". diff --git a/tests/testthat/_snaps/forge-recipe.md b/tests/testthat/_snaps/forge-recipe.md index ed06b38f..4c46f101 100644 --- a/tests/testthat/_snaps/forge-recipe.md +++ b/tests/testthat/_snaps/forge-recipe.md @@ -4,7 +4,7 @@ forge(iris2, x1$blueprint, outcomes = TRUE) Condition Error in `validate_column_names()`: - ! The required column `Species` is missing. + ! The required column "Species" is missing. --- @@ -12,7 +12,7 @@ forge(iris2, x2$blueprint, outcomes = TRUE) Condition Error in `validate_column_names()`: - ! The required column `Species` is missing. + ! The required column "Species" is missing. # missing predictor columns fail appropriately @@ -20,7 +20,7 @@ forge(iris[, 1, drop = FALSE], x$blueprint) Condition Error in `validate_column_names()`: - ! The required column `Sepal.Width` is missing. + ! The required column "Sepal.Width" is missing. --- @@ -28,7 +28,7 @@ forge(iris[, 3, drop = FALSE], x$blueprint) Condition Error in `validate_column_names()`: - ! The required columns `Sepal.Length` and `Sepal.Width` are missing. + ! The required columns "Sepal.Length" and "Sepal.Width" are missing. # novel predictor levels are caught @@ -36,7 +36,7 @@ xx1 <- forge(new, x1$blueprint) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". --- @@ -45,7 +45,7 @@ xx2 <- forge(new, x2$blueprint) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". Warning: ! There are new levels in `f`: NA. @@ -72,7 +72,7 @@ xx1 <- forge(new, x1$blueprint, outcomes = TRUE) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". --- @@ -81,7 +81,7 @@ xx2 <- forge(new, x2$blueprint, outcomes = TRUE) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". # `forge()` will error if required non standard roles are missing @@ -90,7 +90,7 @@ forge(iris, x$blueprint) Condition Error in `validate_column_names()`: - ! The required column `Sepal.Width` is missing. + ! The required column "Sepal.Width" is missing. # `NA` roles are treated as extra roles that are required at `forge()` time @@ -98,7 +98,7 @@ forge(iris, x$blueprint) Condition Error in `validate_column_names()`: - ! The required column `Petal.Length` is missing. + ! The required column "Petal.Length" is missing. # `forge()` is compatible with hardhat 0.2.0 molded blueprints with a basic recipe @@ -106,7 +106,7 @@ forge(new_data, blueprint) Condition Error in `validate_column_names()`: - ! The required column `x` is missing. + ! The required column "x" is missing. # `forge()` is compatible with hardhat 0.2.0 molded blueprints with a recipe with a nonstandard role @@ -114,5 +114,5 @@ forge(new_data, blueprint) Condition Error in `validate_column_names()`: - ! The required column `id` is missing. + ! The required column "id" is missing. diff --git a/tests/testthat/_snaps/forge-xy.md b/tests/testthat/_snaps/forge-xy.md index d1625a90..0183db52 100644 --- a/tests/testthat/_snaps/forge-xy.md +++ b/tests/testthat/_snaps/forge-xy.md @@ -48,7 +48,7 @@ forge(iris[, 1, drop = FALSE], x1$blueprint) Condition Error in `validate_column_names()`: - ! The required column `Sepal.Width` is missing. + ! The required column "Sepal.Width" is missing. --- @@ -56,7 +56,7 @@ forge(iris[, 1, drop = FALSE], x2$blueprint) Condition Error in `validate_column_names()`: - ! The required column `Sepal.Width` is missing. + ! The required column "Sepal.Width" is missing. --- @@ -64,7 +64,7 @@ forge(iris[, 3, drop = FALSE], x1$blueprint) Condition Error in `validate_column_names()`: - ! The required columns `Sepal.Length` and `Sepal.Width` are missing. + ! The required columns "Sepal.Length" and "Sepal.Width" are missing. --- @@ -72,7 +72,7 @@ forge(iris[, 3, drop = FALSE], x2$blueprint) Condition Error in `validate_column_names()`: - ! The required columns `Sepal.Length` and `Sepal.Width` are missing. + ! The required columns "Sepal.Length" and "Sepal.Width" are missing. # novel predictor levels are caught @@ -80,7 +80,7 @@ xx <- forge(new, x$blueprint) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". # novel predictor levels can be ignored @@ -94,7 +94,7 @@ xx1 <- forge(new, x1$blueprint, outcomes = TRUE) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". --- @@ -103,6 +103,6 @@ xx2 <- forge(new, x2$blueprint, outcomes = TRUE) Condition Warning: - Novel level found in column f: "e". + Novel level found in column "f": "e". i The level has been removed, and values have been coerced to "NA". diff --git a/tests/testthat/_snaps/model-offset.md b/tests/testthat/_snaps/model-offset.md index 7cc5769a..7a412737 100644 --- a/tests/testthat/_snaps/model-offset.md +++ b/tests/testthat/_snaps/model-offset.md @@ -4,7 +4,7 @@ mold(~ Sepal.Width + offset(Species), iris) Condition Error in `model_offset()`: - ! Column offset(Species) is tagged as an offset and thus must be numeric, not a object. + ! Column "offset(Species)" is tagged as an offset and thus must be numeric, not a object. # offset columns are stored as predictors @@ -12,5 +12,5 @@ forge(iris2, x$blueprint) Condition Error in `validate_column_names()`: - ! The required column `Sepal.Length` is missing. + ! The required column "Sepal.Length" is missing. From f1ae3992ee4cfdfe6de5fd47972df2991e22ea1e Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Fri, 25 Oct 2024 17:12:41 +0100 Subject: [PATCH 30/36] add pluralization --- R/recompose.R | 3 ++- tests/testthat/_snaps/forge-formula.md | 11 ++++++++++- tests/testthat/test-forge-formula.R | 10 ++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/R/recompose.R b/R/recompose.R index 9592824b..90f0347c 100644 --- a/R/recompose.R +++ b/R/recompose.R @@ -77,7 +77,8 @@ coerce_to_matrix <- function(data, error_call = caller_env()) { message <- c( "{.arg data} must only contain numeric columns.", - i = "These columns aren't numeric: {.val {loc}}." + i = "{cli::qty(length(loc))}{?This/These} column{?s} {?isn't/aren't} + numeric: {.val {loc}}." ) cli::cli_abort(message, call = error_call) diff --git a/tests/testthat/_snaps/forge-formula.md b/tests/testthat/_snaps/forge-formula.md index fa622805..3eb15c14 100644 --- a/tests/testthat/_snaps/forge-formula.md +++ b/tests/testthat/_snaps/forge-formula.md @@ -190,7 +190,16 @@ Condition Error in `recompose()`: ! `data` must only contain numeric columns. - i These columns aren't numeric: "f". + i This column isn't numeric: "f". + +--- + + Code + mold(y ~ f + f_2, dat_2f, blueprint = bp2) + Condition + Error in `recompose()`: + ! `data` must only contain numeric columns. + i These columns aren't numeric: "f" and "f_2". --- diff --git a/tests/testthat/test-forge-formula.R b/tests/testthat/test-forge-formula.R index e1cfdc6c..f37544c6 100644 --- a/tests/testthat/test-forge-formula.R +++ b/tests/testthat/test-forge-formula.R @@ -708,6 +708,16 @@ test_that("can be both missing levels and have new levels", { mold(y ~ f, dat, blueprint = bp2) }) + dat_2f <- data.frame( + y = 1:4, + f = factor(letters[1:4]), + f_2 = factor(letters[5:8]) + ) + + expect_snapshot(error = TRUE, { + mold(y ~ f + f_2, dat_2f, blueprint = bp2) + }) + # Warning for the extra level expect_snapshot(xx <- forge(new, x1$blueprint)) From b12df1ed6ce30cf9aeb1e8cdd531932c0791f30a Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Fri, 25 Oct 2024 18:02:02 +0100 Subject: [PATCH 31/36] found one more `paste0()` for a cli message --- R/blueprint-formula-default.R | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/R/blueprint-formula-default.R b/R/blueprint-formula-default.R index b91e0fc1..c841c9b4 100644 --- a/R/blueprint-formula-default.R +++ b/R/blueprint-formula-default.R @@ -966,10 +966,8 @@ expr_check_no_factorish_in_functions <- function(expr, expr <- as_label(original_expr) message <- c( - paste0( - "Functions involving factors or characters have been detected on the ", - "RHS of `formula`. These are not allowed when `indicators = \"none\"`." - ), + "Functions involving factors or characters have been detected on the + RHS of {.arg formula}. These are not allowed when {.code indicators = \"none\"}.", i = "Functions involving factors were detected for {.val {name}} in {.arg {expr}}." ) @@ -1031,10 +1029,8 @@ expr_check_no_factorish_in_interaction_term <- function(expr, expr <- as_label(expr_original) message <- c( - paste0( - "Interaction terms involving factors or characters have been detected on the ", - "RHS of `formula`. These are not allowed when `indicators = \"none\"`." - ), + "Interaction terms involving factors or characters have been detected on the + RHS of {.arg formula}. These are not allowed when {.code indicators = \"none\"}.", i = "Interactions terms involving factors were detected for {.val {name}} in {.arg {expr}}." ) From 986f7786bbce4914c2fa46626e99ad8595a60c53 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Fri, 25 Oct 2024 18:00:11 +0100 Subject: [PATCH 32/36] reviewer feedback --- R/scream.R | 2 +- tests/testthat/_snaps/forge-formula.md | 22 +++++++++++----------- tests/testthat/_snaps/forge-recipe.md | 8 ++++---- tests/testthat/_snaps/forge-xy.md | 6 +++--- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/R/scream.R b/R/scream.R index 13196886..dbf6c22c 100644 --- a/R/scream.R +++ b/R/scream.R @@ -243,7 +243,7 @@ warn_novel_levels <- function(levels, column) { c( "{cli::qty(n_levels)}Novel level{?s} found in column {.val {column}}: {.val {levels}}.", "i" = "The {cli::qty(n_levels)}level{?s} {?has/have} been removed, - and values have been coerced to {.val NA}." + and values have been coerced to {.cls NA}." ), class = "hardhat_warn_novel_levels", levels = levels, diff --git a/tests/testthat/_snaps/forge-formula.md b/tests/testthat/_snaps/forge-formula.md index 3eb15c14..118ea60f 100644 --- a/tests/testthat/_snaps/forge-formula.md +++ b/tests/testthat/_snaps/forge-formula.md @@ -69,7 +69,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . --- @@ -78,7 +78,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . --- @@ -87,7 +87,7 @@ Condition Warning: Novel levels found in column "f": "e" and "f". - i The levels have been removed, and values have been coerced to "NA". + i The levels have been removed, and values have been coerced to . # novel predictor levels can be ignored @@ -106,7 +106,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . --- @@ -115,7 +115,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . # novel levels are ignored correctly when the new column is a character @@ -134,7 +134,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . --- @@ -143,7 +143,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . # novel outcome levels are always caught, even if `allow_novel_levels = TRUE` @@ -152,7 +152,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . --- @@ -161,7 +161,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . # missing predictor levels are restored silently @@ -208,7 +208,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . # can be both missing levels and have new levels that get ignored @@ -227,5 +227,5 @@ Condition Warning: Novel level found in column "x": "d". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . diff --git a/tests/testthat/_snaps/forge-recipe.md b/tests/testthat/_snaps/forge-recipe.md index 4c46f101..659600fd 100644 --- a/tests/testthat/_snaps/forge-recipe.md +++ b/tests/testthat/_snaps/forge-recipe.md @@ -37,7 +37,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . --- @@ -46,7 +46,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . Warning: ! There are new levels in `f`: NA. i Consider using step_unknown() (`?recipes::step_unknown()`) before `step_dummy()` to handle missing values. @@ -73,7 +73,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . --- @@ -82,7 +82,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . # `forge()` will error if required non standard roles are missing diff --git a/tests/testthat/_snaps/forge-xy.md b/tests/testthat/_snaps/forge-xy.md index 0183db52..6e4ea288 100644 --- a/tests/testthat/_snaps/forge-xy.md +++ b/tests/testthat/_snaps/forge-xy.md @@ -81,7 +81,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . # novel predictor levels can be ignored @@ -95,7 +95,7 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . --- @@ -104,5 +104,5 @@ Condition Warning: Novel level found in column "f": "e". - i The level has been removed, and values have been coerced to "NA". + i The level has been removed, and values have been coerced to . From 456c3059656f064380c695928c13fcbb6b14aafe Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Fri, 25 Oct 2024 19:03:20 +0100 Subject: [PATCH 33/36] more usage of custom checkers --- R/intercept.R | 10 +--------- R/quantile-pred.R | 7 +------ tests/testthat/_snaps/intercept.md | 2 +- 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/R/intercept.R b/R/intercept.R index fa6d07e5..56bf6895 100644 --- a/R/intercept.R +++ b/R/intercept.R @@ -22,15 +22,7 @@ #' add_intercept_column(as.matrix(mtcars)) #' @export add_intercept_column <- function(data, name = "(Intercept)") { - ok <- is.data.frame(data) || is.matrix(data) - - if (!ok) { - cli::cli_abort( - "{.arg data} must be a data frame or matrix to add an intercept column, - not {.obj_type_friendly {data}}." - ) - } - + check_data_frame_or_matrix(data) check_name(name) if (name %in% colnames(data)) { diff --git a/R/quantile-pred.R b/R/quantile-pred.R index 8a26fff5..941787ef 100644 --- a/R/quantile-pred.R +++ b/R/quantile-pred.R @@ -134,12 +134,7 @@ obj_print_footer.quantile_pred <- function(x, digits = 3, ...) { # Checking functions check_quantile_pred_inputs <- function(values, levels, call = caller_env()) { - if (!is.matrix(values)) { - cli::cli_abort( - "{.arg values} must be a {.cls matrix}, not {.obj_type_friendly {values}}.", - call = call - ) - } + check_inherits(values, "matrix", call = call) num_lvls <- length(levels) diff --git a/tests/testthat/_snaps/intercept.md b/tests/testthat/_snaps/intercept.md index 65eba53b..3cc992d7 100644 --- a/tests/testthat/_snaps/intercept.md +++ b/tests/testthat/_snaps/intercept.md @@ -29,5 +29,5 @@ add_intercept_column(1) Condition Error in `add_intercept_column()`: - ! `data` must be a data frame or matrix to add an intercept column, not a number. + ! `data` must be a data frame or a matrix, not the number 1. From 3b61e62e513910d9f57d2a428e0239cd6b3b42ea Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Fri, 25 Oct 2024 19:04:25 +0100 Subject: [PATCH 34/36] consistent indentation --- R/blueprint-formula-default.R | 5 +++- R/quantile-pred.R | 10 ++++--- R/util.R | 7 +++-- R/validation.R | 54 ++++++++++++++++++++--------------- 4 files changed, 45 insertions(+), 31 deletions(-) diff --git a/R/blueprint-formula-default.R b/R/blueprint-formula-default.R index c841c9b4..7a277c12 100644 --- a/R/blueprint-formula-default.R +++ b/R/blueprint-formula-default.R @@ -878,7 +878,10 @@ check_levels <- function(x, } if (!all(map_lgl(x, is.character))) { - cli::cli_abort("{.arg {arg}} must only contain character vectors.", call = call) + cli::cli_abort( + "{.arg {arg}} must only contain character vectors.", + call = call + ) } invisible(NULL) diff --git a/R/quantile-pred.R b/R/quantile-pred.R index 941787ef..9695c577 100644 --- a/R/quantile-pred.R +++ b/R/quantile-pred.R @@ -166,10 +166,12 @@ check_quantile_levels <- function(levels, call = rlang::caller_env()) { redund <- levels[is_dup] redund <- unique(redund) redund <- signif(redund, digits = 5) - cli::cli_abort(c( - "Quantile levels must be unique.", - i = "The following {cli::qty(length(redund))}value{?s} {?was/were} repeated: - {redund}."), + cli::cli_abort( + c( + "Quantile levels must be unique.", + i = "The following {cli::qty(length(redund))}value{?s} {?was/were} + repeated: {redund}." + ), call = call ) } diff --git a/R/util.R b/R/util.R index 76ee3b4d..a491969d 100644 --- a/R/util.R +++ b/R/util.R @@ -34,8 +34,8 @@ get_all_predictors <- function(formula, data) { extra_predictors <- setdiff(predictors, names(data)) if (length(extra_predictors) > 0) { cli::cli_abort( - "The following predictor{?s} {?was/were} not found in - {.arg data}: {.val {extra_predictors}}." + "The following predictor{?s} {?was/were} not found in {.arg data}: + {.val {extra_predictors}}." ) } @@ -61,7 +61,8 @@ get_all_outcomes <- function(formula, data) { if (length(extra_outcomes) > 0) { cli::cli_abort( "The following outcome{?s} {?was/were} not found in {.arg data}: - {.val {extra_outcomes}}.") + {.val {extra_outcomes}}." + ) } outcomes diff --git a/R/validation.R b/R/validation.R index 1b5be255..15c59975 100644 --- a/R/validation.R +++ b/R/validation.R @@ -113,11 +113,13 @@ validate_outcomes_are_numeric <- function(outcomes) { if (!check$ok) { bad_msg <- style_bad_classes(check$bad_classes) - cli::cli_abort(c( - "All outcomes must be numeric.", - "i" = "{cli::qty(length(check$bad_classes))}The following {?is/are} not:", - bad_msg - )) + cli::cli_abort( + c( + "All outcomes must be numeric.", + "i" = "{cli::qty(length(check$bad_classes))}The following {?is/are} not:", + bad_msg + ) + ) } invisible(outcomes) @@ -196,11 +198,13 @@ validate_outcomes_are_factors <- function(outcomes) { if (!check$ok) { bad_msg <- style_bad_classes(check$bad_classes) - cli::cli_abort(c( - "All outcomes must be factors.", - "i" = "{cli::qty(length(check$bad_classes))}The following {?is/are} not:", - bad_msg - )) + cli::cli_abort( + c( + "All outcomes must be factors.", + "i" = "{cli::qty(length(check$bad_classes))}The following {?is/are} not:", + bad_msg + ) + ) } invisible(outcomes) @@ -279,12 +283,14 @@ validate_outcomes_are_binary <- function(outcomes) { bad_msg <- glue::glue("{bad_col}: {check$num_levels}") - cli::cli_abort(c( - "The outcome must be binary.", - "i" = "{cli::qty(length(bad_col))}The following number of levels - {?was/were} found:", - bad_msg - )) + cli::cli_abort( + c( + "The outcome must be binary.", + "i" = "{cli::qty(length(bad_col))}The following number of levels + {?was/were} found:", + bad_msg + ) + ) } invisible(outcomes) @@ -369,11 +375,13 @@ validate_predictors_are_numeric <- function(predictors) { if (!check$ok) { bad_msg <- style_bad_classes(check$bad_classes) - cli::cli_abort(c( - "All predictors must be numeric.", - "i" = "{cli::qty(length(check$bad_classes))}The following {?is/are} not:", - bad_msg - )) + cli::cli_abort( + c( + "All predictors must be numeric.", + "i" = "{cli::qty(length(check$bad_classes))}The following {?is/are} not:", + bad_msg + ) + ) } invisible(predictors) @@ -687,8 +695,8 @@ validate_no_formula_duplication <- function(formula, original = FALSE) { if (!check$ok) { cli::cli_abort( c( - "Terms must not be duplicated on the left- and right-hand side - of the {.arg formula}.", + "Terms must not be duplicated on the left- and right-hand side of the + {.arg formula}.", "i" = "The following duplicated term{?s} {?was/were} found: {.val {check$duplicates}}" ) From 89b81cc67c8813db6b6f83937ec2170cb21fa277 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Fri, 25 Oct 2024 19:05:30 +0100 Subject: [PATCH 35/36] one more friendly object type --- R/encoding.R | 2 +- tests/testthat/_snaps/encoding.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/encoding.R b/R/encoding.R index 92921c43..46a57cf3 100644 --- a/R/encoding.R +++ b/R/encoding.R @@ -31,7 +31,7 @@ #' fct_encode_one_hot(factor(sample(letters[1:4], 10, TRUE))) fct_encode_one_hot <- function(x) { if (!is.factor(x)) { - cli::cli_abort("{.arg x} must be a factor.") + cli::cli_abort("{.arg x} must be a factor, not {.obj_type_friendly {x}}.") } row_names <- names(x) diff --git a/tests/testthat/_snaps/encoding.md b/tests/testthat/_snaps/encoding.md index 2c525592..744b534c 100644 --- a/tests/testthat/_snaps/encoding.md +++ b/tests/testthat/_snaps/encoding.md @@ -12,5 +12,5 @@ fct_encode_one_hot(1) Condition Error in `fct_encode_one_hot()`: - ! `x` must be a factor. + ! `x` must be a factor, not a number. From 35c0daaeccaa85554ff2c1820c7d8349f02a520b Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Fri, 25 Oct 2024 19:05:38 +0100 Subject: [PATCH 36/36] one more styling --- R/blueprint-formula-default.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/blueprint-formula-default.R b/R/blueprint-formula-default.R index 7a277c12..63b81ee8 100644 --- a/R/blueprint-formula-default.R +++ b/R/blueprint-formula-default.R @@ -1071,7 +1071,7 @@ expr_check_no_interactions <- function(expr, error_call) { expr <- as_label(expr) message <- c( - "Interaction terms can't be specified on the LHS of `formula`.", + "Interaction terms can't be specified on the LHS of {.arg formula}.", i = "The following interaction term was found: {.arg {expr}}." )