Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rsample cli errors update for #510 #531

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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, #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).

Expand Down
11 changes: 8 additions & 3 deletions R/permutations.R
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,19 @@ 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)) {
rlang::abort("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))) {
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(
"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)
Expand Down
4 changes: 2 additions & 2 deletions R/reg_intervals.R
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ reg_intervals <-
} else {
times <- times[1]
if (!is.numeric(times)) {
rlang::abort("'times' should be a single integer.")
cli_abort("{.arg times} should be a single integer.")
}
}

if (length(alpha) != 1 || !is.numeric(alpha)) {
abort("`alpha` must be a single numeric value.")
cli_abort("{.arg alpha} must be a single numeric value.")
}

if (model_fn %in% c("survreg", "coxph")) {
Expand Down
10 changes: 10 additions & 0 deletions tests/testthat/_snaps/permutations.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@
Error in `as.data.frame()`:
! There is no assessment data set for an `rsplit` object with class <perm_split>.

# 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
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-permutations.R
Original file line number Diff line number Diff line change
Expand Up @@ -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", {
Expand Down
Loading