From 0542408a346440db66237838b8181aaff8309d1a Mon Sep 17 00:00:00 2001 From: James Wade Date: Thu, 15 Aug 2024 09:38:26 -0700 Subject: [PATCH 1/5] Use cli erros in rsplit.R Fixes #512 --- R/rsplit.R | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/R/rsplit.R b/R/rsplit.R index 96234934..c13b7333 100644 --- a/R/rsplit.R +++ b/R/rsplit.R @@ -1,20 +1,20 @@ rsplit <- function(data, in_id, out_id) { if (!is.data.frame(data) & !is.matrix(data)) { - rlang::abort("`data` must be a data frame.") + cli::cli_abort("`data` must be a data frame.") } if (!is.integer(in_id) | any(in_id < 1)) { - rlang::abort("`in_id` must be a positive integer vector.") + cli::cli_abort("`in_id` must be a positive integer vector.") } if (!all(is.na(out_id))) { if (!is.integer(out_id) | any(out_id < 1)) { - rlang::abort("`out_id` must be a positive integer vector.") + cli::cli_abort("`out_id` must be a positive integer vector.") } } if (length(in_id) == 0) { - rlang::abort("At least one row should be selected for the analysis set.") + cli::cli_abort("At least one row should be selected for the analysis set.") } structure( @@ -88,13 +88,13 @@ as.data.frame.rsplit <- data = "analysis", ...) { if (!is.null(row.names)) { - rlang::warn(paste0( + cli::cli_warn(paste0( "`row.names` is kept for consistency with the underlying class but ", "non-NULL values will be ignored." )) } if (optional) { - rlang::warn(paste0( + cli::cli_warn(paste0( "`optional` is kept for consistency with the underlying class but ", "TRUE values will be ignored." )) @@ -107,7 +107,7 @@ as.data.frame.rsplit <- "There is no assessment data set for an `rsplit` object", " with class `", rsplit_class, "`." ) - rlang::abort(msg) + cli::cli_abort(msg) } ind <- as.integer(x, data = data, ...) permuted_col <- vctrs::vec_slice(x$data, ind) %>% From f47b9df40552dbce6e4d2e91276417ab77d513f2 Mon Sep 17 00:00:00 2001 From: James Wade Date: Thu, 15 Aug 2024 10:16:21 -0700 Subject: [PATCH 2/5] drop cli:: from cli_abort, use .arg, and remove paste0 --- R/rsplit.R | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/R/rsplit.R b/R/rsplit.R index c13b7333..a82b1562 100644 --- a/R/rsplit.R +++ b/R/rsplit.R @@ -1,20 +1,20 @@ rsplit <- function(data, in_id, out_id) { if (!is.data.frame(data) & !is.matrix(data)) { - cli::cli_abort("`data` must be a data frame.") + cli_abort("{.arg `data`} must be a data frame.") } if (!is.integer(in_id) | any(in_id < 1)) { - cli::cli_abort("`in_id` must be a positive integer vector.") + cli_abort("{.arg in_id} must be a positive integer vector.") } if (!all(is.na(out_id))) { if (!is.integer(out_id) | any(out_id < 1)) { - cli::cli_abort("`out_id` must be a positive integer vector.") + cli_abort("{.arg out_id} must be a positive integer vector.") } } if (length(in_id) == 0) { - cli::cli_abort("At least one row should be selected for the analysis set.") + cli_abort("At least one row should be selected for the analysis set.") } structure( @@ -88,26 +88,25 @@ as.data.frame.rsplit <- data = "analysis", ...) { if (!is.null(row.names)) { - cli::cli_warn(paste0( - "`row.names` is kept for consistency with the underlying class but ", - "non-NULL values will be ignored." - )) + cli::cli_warn( + "{.arg row.names} is kept for consistency with the underlying class but + non-NULL values will be ignored." + ) } if (optional) { - cli::cli_warn(paste0( - "`optional` is kept for consistency with the underlying class but ", - "TRUE values will be ignored." - )) + cli::cli_warn( + "{.arg optional} is kept for consistency with the underlying class but + TRUE values will be ignored." + ) } if (!is.null(x$col_id)) { if (identical(data, "assessment")) { rsplit_class <- class(x)[[1]] - msg <- paste0( - "There is no assessment data set for an `rsplit` object", - " with class `", rsplit_class, "`." + cli_abort( + "There is no assessment data set for an {.arg rsplit} object + with class {rsplit_class}." ) - cli::cli_abort(msg) } ind <- as.integer(x, data = data, ...) permuted_col <- vctrs::vec_slice(x$data, ind) %>% From 635ff72fbbff7c8797cf1940b12ad2ccc885abb0 Mon Sep 17 00:00:00 2001 From: James Wade Date: Thu, 15 Aug 2024 10:33:35 -0700 Subject: [PATCH 3/5] remove extra backticks --- R/rsplit.R | 2 +- tests/testthat/_snaps/permutations.new.md | 30 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 tests/testthat/_snaps/permutations.new.md diff --git a/R/rsplit.R b/R/rsplit.R index a82b1562..d99a98cb 100644 --- a/R/rsplit.R +++ b/R/rsplit.R @@ -1,6 +1,6 @@ rsplit <- function(data, in_id, out_id) { if (!is.data.frame(data) & !is.matrix(data)) { - cli_abort("{.arg `data`} must be a data frame.") + cli_abort("{.arg data} must be a data frame.") } if (!is.integer(in_id) | any(in_id < 1)) { diff --git a/tests/testthat/_snaps/permutations.new.md b/tests/testthat/_snaps/permutations.new.md new file mode 100644 index 00000000..515f099d --- /dev/null +++ b/tests/testthat/_snaps/permutations.new.md @@ -0,0 +1,30 @@ +# no assessment set + + Code + assessment(xx$splits[[1]]) + Condition + Error in `as.data.frame()`: + ! There is no assessment data set for an `rsplit` object with class perm_split. + +# printing + + Code + permutations(mtcars, 1) + Output + # Permutation sampling + # Permuted columns: [mpg] + # A tibble: 25 x 2 + splits id + + 1 Permutations01 + 2 Permutations02 + 3 Permutations03 + 4 Permutations04 + 5 Permutations05 + 6 Permutations06 + 7 Permutations07 + 8 Permutations08 + 9 Permutations09 + 10 Permutations10 + # i 15 more rows + From 0093a28c3eb7c7aa3d883253f14f153e5b31309e Mon Sep 17 00:00:00 2001 From: James Wade Date: Thu, 15 Aug 2024 10:45:49 -0700 Subject: [PATCH 4/5] update arg names and accept new tests --- R/rsplit.R | 2 +- tests/testthat/_snaps/permutations.md | 2 +- tests/testthat/_snaps/permutations.new.md | 30 ----------------------- 3 files changed, 2 insertions(+), 32 deletions(-) delete mode 100644 tests/testthat/_snaps/permutations.new.md diff --git a/R/rsplit.R b/R/rsplit.R index d99a98cb..744e9d9e 100644 --- a/R/rsplit.R +++ b/R/rsplit.R @@ -105,7 +105,7 @@ as.data.frame.rsplit <- rsplit_class <- class(x)[[1]] cli_abort( "There is no assessment data set for an {.arg rsplit} object - with class {rsplit_class}." + with class {.cls {rsplit_class}}." ) } ind <- as.integer(x, data = data, ...) diff --git a/tests/testthat/_snaps/permutations.md b/tests/testthat/_snaps/permutations.md index 94276f18..1f1b3571 100644 --- a/tests/testthat/_snaps/permutations.md +++ b/tests/testthat/_snaps/permutations.md @@ -4,7 +4,7 @@ assessment(xx$splits[[1]]) Condition Error in `as.data.frame()`: - ! There is no assessment data set for an `rsplit` object with class `perm_split`. + ! There is no assessment data set for an `rsplit` object with class . # printing diff --git a/tests/testthat/_snaps/permutations.new.md b/tests/testthat/_snaps/permutations.new.md deleted file mode 100644 index 515f099d..00000000 --- a/tests/testthat/_snaps/permutations.new.md +++ /dev/null @@ -1,30 +0,0 @@ -# no assessment set - - Code - assessment(xx$splits[[1]]) - Condition - Error in `as.data.frame()`: - ! There is no assessment data set for an `rsplit` object with class perm_split. - -# printing - - Code - permutations(mtcars, 1) - Output - # Permutation sampling - # Permuted columns: [mpg] - # A tibble: 25 x 2 - splits id - - 1 Permutations01 - 2 Permutations02 - 3 Permutations03 - 4 Permutations04 - 5 Permutations05 - 6 Permutations06 - 7 Permutations07 - 8 Permutations08 - 9 Permutations09 - 10 Permutations10 - # i 15 more rows - From 0e87add6292faf91565fadafcf0e35134765ea95 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Thu, 15 Aug 2024 11:27:05 -0700 Subject: [PATCH 5/5] add James to NEWS --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 2e678b33..bd4a0214 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). +* Started moving error messages to cli (#499, #502). With contributions from @JamesHWade (#518). ## Bug fixes