From fbc2e2e9debdacd99177e905295ae32a5b0c63cd Mon Sep 17 00:00:00 2001 From: Priyata Kalra Date: Thu, 29 Aug 2024 13:02:07 +0200 Subject: [PATCH 1/3] Rsample cli errors update for 510 --- R/permutations.R | 11 ++++++++--- R/reg_intervals.R | 10 ++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/R/permutations.R b/R/permutations.R index 8e2df6b0..64e17b0b 100644 --- a/R/permutations.R +++ b/R/permutations.R @@ -58,10 +58,15 @@ permutations <- function(data, col_id <- tidyselect::eval_select(permute, data) if (identical(length(col_id), 0L)) { - rlang::abort("You must specify at least one column to permute!") + cli_abort("{.strong You must specify at least one column to permute!}") } else if (identical(length(col_id), ncol(data))) { - rlang::abort("You have selected all columns to permute. This effectively reorders the rows in the original data without changing the data structure. Please select fewer columns to permute.") - } + cli_abort(c( + "!" = "{.emph {.strong You have selected all columns to permute.}}", + "x" = "This effectively reorders the rows in the original data without changing the data structure.", + "i" = "To achieve meaningful permutation:", + "*" = "{.field Select fewer columns} to permute.", + ">" = "Ideal: Choose specific columns that are relevant to your analysis.")) + } split_objs <- perm_splits(data, times) diff --git a/R/reg_intervals.R b/R/reg_intervals.R index 50a0cf44..ef2faed8 100644 --- a/R/reg_intervals.R +++ b/R/reg_intervals.R @@ -54,12 +54,18 @@ reg_intervals <- } else { times <- times[1] if (!is.numeric(times)) { - rlang::abort("'times' should be a single integer.") + cli_abort(c( + "x" = "{.arg times} should be a single integer.", + "i" = "You provided {.val {times}}." + )) } } if (length(alpha) != 1 || !is.numeric(alpha)) { - abort("`alpha` must be a single numeric value.") + cli_abort(c( + "x" = "{.arg alpha} must be a single numeric value.", + "i" = "Please ensure that {.arg alpha} is a numeric value and not a vector or other type." + )) } if (model_fn %in% c("survreg", "coxph")) { From 6281f3972c98c036bfdb6ff577ebdb1496015288 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Tue, 10 Sep 2024 14:27:03 +0100 Subject: [PATCH 2/3] more in keeping with other error messages --- R/permutations.R | 16 ++++++++-------- R/reg_intervals.R | 10 ++-------- tests/testthat/_snaps/permutations.md | 10 ++++++++++ tests/testthat/test-permutations.R | 2 +- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/R/permutations.R b/R/permutations.R index 64e17b0b..47fe3aba 100644 --- a/R/permutations.R +++ b/R/permutations.R @@ -53,20 +53,20 @@ permutations <- function(data, permute <- rlang::enquo(permute) if (is.null(permute)) { - rlang::abort("You must specify at least one column to permute!") + cli_abort("You must specify at least one column to permute.") } col_id <- tidyselect::eval_select(permute, data) if (identical(length(col_id), 0L)) { - cli_abort("{.strong You must specify at least one column to permute!}") + cli_abort("You must specify at least one column to permute.") } else if (identical(length(col_id), ncol(data))) { cli_abort(c( - "!" = "{.emph {.strong You have selected all columns to permute.}}", - "x" = "This effectively reorders the rows in the original data without changing the data structure.", - "i" = "To achieve meaningful permutation:", - "*" = "{.field Select fewer columns} to permute.", - ">" = "Ideal: Choose specific columns that are relevant to your analysis.")) - } + "You have selected all columns to permute.", + "i" = "This effectively reorders the rows in the original data without + changing the data structure.", + ">" = "Please select fewer columns to permute." + )) + } split_objs <- perm_splits(data, times) diff --git a/R/reg_intervals.R b/R/reg_intervals.R index ef2faed8..7da4fada 100644 --- a/R/reg_intervals.R +++ b/R/reg_intervals.R @@ -54,18 +54,12 @@ reg_intervals <- } else { times <- times[1] if (!is.numeric(times)) { - cli_abort(c( - "x" = "{.arg times} should be a single integer.", - "i" = "You provided {.val {times}}." - )) + cli_abort("{.arg times} should be a single integer.") } } if (length(alpha) != 1 || !is.numeric(alpha)) { - cli_abort(c( - "x" = "{.arg alpha} must be a single numeric value.", - "i" = "Please ensure that {.arg alpha} is a numeric value and not a vector or other type." - )) + cli_abort("{.arg alpha} must be a single numeric value.") } if (model_fn %in% c("survreg", "coxph")) { diff --git a/tests/testthat/_snaps/permutations.md b/tests/testthat/_snaps/permutations.md index 1f1b3571..84233a4f 100644 --- a/tests/testthat/_snaps/permutations.md +++ b/tests/testthat/_snaps/permutations.md @@ -6,6 +6,16 @@ Error in `as.data.frame()`: ! There is no assessment data set for an `rsplit` object with class . +# bad args + + Code + permutations(mtcars, everything()) + Condition + Error in `permutations()`: + ! You have selected all columns to permute. + i This effectively reorders the rows in the original data without changing the data structure. + > Please select fewer columns to permute. + # printing Code diff --git a/tests/testthat/test-permutations.R b/tests/testthat/test-permutations.R index e2cd4702..91e3dfdf 100644 --- a/tests/testthat/test-permutations.R +++ b/tests/testthat/test-permutations.R @@ -37,7 +37,7 @@ test_that("bad args", { expect_error(permutations(mtcars)) # no columns specified expect_error(permutations(mtcars, foo)) # column doesn't exist expect_error(permutations(mtcars, start_with("z"))) # column doesn't exist - expect_error(permutations(mtcars, everything())) # all columns + expect_snapshot(error = TRUE, {permutations(mtcars, everything())}) # all columns }) test_that("printing", { From 71511bf46223b66658922a2baf889de60ee442cc Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Tue, 10 Sep 2024 14:28:58 +0100 Subject: [PATCH 3/3] Update acknowledgment --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 4960e310..dcd0ac7f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,7 +2,7 @@ * The new `inner_split()` function and its methods for various resamples is for usage in tune to create a inner resample of the analysis set to fit the preprocessor and model on one part and the post-processor on the other part (#483, #488, #489). -* Started moving error messages to cli (#499, #502). With contributions from @PriKalra (#523, #526, #528, #530) and @JamesHWade (#518). +* Started moving error messages to cli (#499, #502). With contributions from @PriKalra (#523, #526, #528, #530, #531) and @JamesHWade (#518). * Fixed example for `nested_cv()` (@seb09, #520).