From cba51be1ebf08e1a4042ebbba1696bcbfb58311e Mon Sep 17 00:00:00 2001 From: Priyata Kalra Date: Mon, 19 Aug 2024 15:43:24 +0200 Subject: [PATCH 1/6] Added cli_abort and changed some formating. Fixes 511 --- R/rset.R | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/R/rset.R b/R/rset.R index ee8e9d09..751cb4fe 100644 --- a/R/rset.R +++ b/R/rset.R @@ -18,7 +18,7 @@ new_rset <- function(splits, ids, attrib = NULL, ids <- tibble(id = ids) } else { if (!all(grepl("^id", names(ids)))) { - rlang::abort("The `ids` tibble column names should start with 'id'.") + cli_abort("The {.code id} tibble column names should start with 'id'.") } } either_type <- function(x) { @@ -26,15 +26,15 @@ new_rset <- function(splits, ids, attrib = NULL, } ch_check <- vapply(ids, either_type, c(logical = TRUE)) if (!all(ch_check)) { - rlang::abort("All ID columns should be character or factor vectors.") + cli_abort("{.strong All} ID columns should be character or factor {.field vectors}.") } if (!is_tibble(splits)) { splits <- tibble(splits = splits) } else { if (ncol(splits) > 1 | names(splits)[1] != "splits") { - rlang::abort( - "The `splits` tibble should have a single column named `splits`." + cli_abort( + "The {.var splits} tibble should have a single column named {.code splits}." ) } } @@ -42,11 +42,11 @@ new_rset <- function(splits, ids, attrib = NULL, where_rsplits <- vapply(splits[["splits"]], is_rsplit, logical(1)) if (!all(where_rsplits)) { - rlang::abort("Each element of `splits` must be an `rsplit` object.") + cli_abort("Each element of {.var splits} must be an {.var rsplit} object.") } if (nrow(ids) != nrow(splits)) { - rlang::abort("Split and ID vectors have different lengths.") + cli_abort("Split and ID vectors have different lengths.") } # Create another element to the splits that is a tibble containing @@ -64,7 +64,7 @@ new_rset <- function(splits, ids, attrib = NULL, if (!is.null(attrib)) { if (any(names(attrib) == "")) { - rlang::abort("`attrib` should be a fully named list.") + cli_abort("{.var attrib} should be a fully named {.field list}.") } for (i in names(attrib)) { attr(res, i) <- attrib[[i]] From 74e61dd5cfef0f50e9651fb0491ba53e4533c1cc Mon Sep 17 00:00:00 2001 From: Priyata Date: Thu, 29 Aug 2024 12:19:17 +0200 Subject: [PATCH 2/6] Update R/rset.R Co-authored-by: Simon P. Couch --- R/rset.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/rset.R b/R/rset.R index 751cb4fe..281505d5 100644 --- a/R/rset.R +++ b/R/rset.R @@ -64,7 +64,7 @@ new_rset <- function(splits, ids, attrib = NULL, if (!is.null(attrib)) { if (any(names(attrib) == "")) { - cli_abort("{.var attrib} should be a fully named {.field list}.") + cli_abort("{.arg attrib} should be a fully named list.") } for (i in names(attrib)) { attr(res, i) <- attrib[[i]] From 70b6b9d741074b823a66111a75aa522375349c69 Mon Sep 17 00:00:00 2001 From: Priyata Date: Thu, 29 Aug 2024 12:19:24 +0200 Subject: [PATCH 3/6] Update R/rset.R Co-authored-by: Simon P. Couch --- R/rset.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/rset.R b/R/rset.R index 281505d5..0defe9f6 100644 --- a/R/rset.R +++ b/R/rset.R @@ -42,7 +42,7 @@ new_rset <- function(splits, ids, attrib = NULL, where_rsplits <- vapply(splits[["splits"]], is_rsplit, logical(1)) if (!all(where_rsplits)) { - cli_abort("Each element of {.var splits} must be an {.var rsplit} object.") + cli_abort("Each element of {.arg splits} must be an {.cls rsplit} object.") } if (nrow(ids) != nrow(splits)) { From e1dc2f2ce176b6024d46f274cd2b9765205cd9d0 Mon Sep 17 00:00:00 2001 From: Priyata Date: Thu, 29 Aug 2024 12:19:31 +0200 Subject: [PATCH 4/6] Update R/rset.R Co-authored-by: Simon P. Couch --- R/rset.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/rset.R b/R/rset.R index 0defe9f6..061b57c2 100644 --- a/R/rset.R +++ b/R/rset.R @@ -26,7 +26,7 @@ new_rset <- function(splits, ids, attrib = NULL, } ch_check <- vapply(ids, either_type, c(logical = TRUE)) if (!all(ch_check)) { - cli_abort("{.strong All} ID columns should be character or factor {.field vectors}.") + cli_abort("{.strong All} ID columns should be character or factor vectors.") } if (!is_tibble(splits)) { From 3faebaf3fb463c76ae9e0ae32f7421b6b87d3b12 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Mon, 9 Sep 2024 20:16:04 +0100 Subject: [PATCH 5/6] update test --- tests/testthat/_snaps/rset.md | 8 ++++++++ tests/testthat/test-rset.R | 5 +---- 2 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 tests/testthat/_snaps/rset.md diff --git a/tests/testthat/_snaps/rset.md b/tests/testthat/_snaps/rset.md new file mode 100644 index 00000000..60dc8c41 --- /dev/null +++ b/tests/testthat/_snaps/rset.md @@ -0,0 +1,8 @@ +# bad args + + Code + new_rset(list(1), "x") + Condition + Error in `new_rset()`: + ! Each element of `splits` must be an object. + diff --git a/tests/testthat/test-rset.R b/tests/testthat/test-rset.R index 32d5e8cc..c09e0cab 100644 --- a/tests/testthat/test-rset.R +++ b/tests/testthat/test-rset.R @@ -8,10 +8,7 @@ test_that("bad args", { expect_error( new_rset(car_folds$splits, car_folds$splits) ) - expect_error( - new_rset(list(1), "x"), - "must be an `rsplit` object" - ) + expect_snapshot(error = TRUE, {new_rset(list(1), "x")}) args <- list(a = 1, b = 2, 3) expect_error( new_rset( From 972acb5c73bbdc890fa7b010c0261472aae6f4d2 Mon Sep 17 00:00:00 2001 From: Hannah Frick Date: Mon, 9 Sep 2024 20:18:29 +0100 Subject: [PATCH 6/6] Update acknowledgment --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index c18dc579..4960e310 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) and @JamesHWade (#518). +* Started moving error messages to cli (#499, #502). With contributions from @PriKalra (#523, #526, #528, #530) and @JamesHWade (#518). * Fixed example for `nested_cv()` (@seb09, #520).