Skip to content

Commit

Permalink
Merge pull request #530 from PriKalra/cliErrorRsetUpdate511
Browse files Browse the repository at this point in the history
Added cli_abort and changed some formatting.
  • Loading branch information
hfrick authored Sep 9, 2024
2 parents 9fe256c + 972acb5 commit 4f81425
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 12 deletions.
2 changes: 1 addition & 1 deletion NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).

Expand Down
14 changes: 7 additions & 7 deletions R/rset.R
Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,35 @@ 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) {
is.character(x) | is.factor(x)
}
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 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}."
)
}
}

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 {.arg splits} must be an {.cls 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
Expand All @@ -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("{.arg attrib} should be a fully named list.")
}
for (i in names(attrib)) {
attr(res, i) <- attrib[[i]]
Expand Down
8 changes: 8 additions & 0 deletions tests/testthat/_snaps/rset.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# bad args

Code
new_rset(list(1), "x")
Condition
Error in `new_rset()`:
! Each element of `splits` must be an <rsplit> object.

5 changes: 1 addition & 4 deletions tests/testthat/test-rset.R
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit 4f81425

Please sign in to comment.