Add missing snapshots - part 1 #1383
Conversation
| if (length(col_name) > 1) { | ||
| cli::cli_abort(c( | ||
| x = "The selector should select at most a single variable.", | ||
| i = "The following {length(col_name)} were selected: \\ | ||
| {.and {.var {col_name}}}." | ||
| )) | ||
| } |
There was a problem hiding this comment.
this is unreachable because we are already checking for this in step_count() in line 83
| if (!is.numeric(var)) { | ||
| cli::cli_abort( | ||
| "{.arg var} must be a numeric vector, not {.obj_type_friendly {var}}." | ||
| ) | ||
| } |
There was a problem hiding this comment.
this is no longer needed as we do the checking with `check_type()
| if (!is.factor(x)) { | ||
| cli::cli_abort( | ||
| "{.arg x} must be a factor, not {.obj_type_friendly {x}}.", | ||
| .internal = TRUE | ||
| ) | ||
| } |
There was a problem hiding this comment.
didn't feel like this was needed, since adjust_levels_min_max() is used on the output of cut() which should always be a factor.
I checked, and if cut() were to produce a non-factor, then other errors go off
| if (is.null(levels_values)) { | ||
| cli::cli_abort("Factor level values not recorded in {.var col_name}.") | ||
| } | ||
|
|
There was a problem hiding this comment.
unreachable as we require that step_dummy() is done on factors
| @@ -165,23 +163,6 @@ prep.step_dummy_multi_choice <- function(x, training, info = NULL, ...) { | |||
| ) | |||
| } | |||
|
|
|||
| multi_dummy_check_type <- function(dat, call = rlang::caller_env()) { | |||
There was a problem hiding this comment.
no longer needed due to check_type()
| if (!is.numeric(trim) || length(trim) != 1L) { | ||
| cli::cli_abort("{.arg trim} must be numeric of length one.") | ||
| } |
There was a problem hiding this comment.
technically unreachable, since there is a if (trim == 0) {} above. but also isn't needed since it should be a rlang type checked earlier anyways
| if (is.complex(x)) { | ||
| cli::cli_abort("Trimmed means are not defined for complex data.") | ||
| } |
There was a problem hiding this comment.
can't be reached since check_type() doesn't allow complex data
| @@ -162,7 +162,8 @@ prep.step_interact <- function(x, training, info = NULL, ...) { | |||
| ) | |||
| if (!is_formula(tmp_terms)) { | |||
| cli::cli_abort( | |||
| "{.arg terms} must be a formula. Not {.obj_type_friendly {term}}." | |||
| "{.arg terms} must be a formula. Not {.obj_type_friendly {term}}.", | |||
| .internal = TRUE | |||
There was a problem hiding this comment.
I have in a couple of places added .internal = TRUE. I did this because I was not able to find a reprex, but it felt reasonable that it could happen.
|
|
||
| # Get existing levels and their factor type (i.e. ordered) | ||
| objects <- lapply(training[, col_names], get_existing_values) | ||
| # Check to make sure that no ordered levels are provided | ||
| order_check <- map_lgl(objects, attr, "is_ordered") | ||
| if (any(order_check)) { |
There was a problem hiding this comment.
isn't needed as we can do it with check_type()
| @@ -118,7 +118,7 @@ step_impute_knn <- | |||
| if (length(options) > 0) { | |||
| if (any(!(opt_nms %in% c("eps", "nthread")))) { | |||
There was a problem hiding this comment.
I'm planning to add a helper function for this as we pass through additional arguments in a list a few places
#1269
| @@ -165,23 +163,6 @@ prep.step_dummy_multi_choice <- function(x, training, info = NULL, ...) { | |||
| ) | |||
| } | |||
|
|
|||
| multi_dummy_check_type <- function(dat, call = rlang::caller_env()) { | |||
tests/testthat/_snaps/pls.md
Outdated
| options = list(kernel = "wrong")) %>% prep() | ||
| Condition | ||
| Warning: | ||
| `step_pls()` failed with error: Error in mixOmics::pls(X = x, Y = y, ncomp = 2, kernel = ~"wrong") : unused argument (kernel = ~"wrong") . |
There was a problem hiding this comment.
Lots of formatting here that I suspect would be pared back by just setting parent
There was a problem hiding this comment.
talk more here: #1386
Co-authored-by: Simon P. Couch <simonpatrickcouch@gmail.com>
|
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex https://reprex.tidyverse.org) and link to this issue. |
I didn't get all of them done in this round. and it wouldn't hurt to split them up