-
Notifications
You must be signed in to change notification settings - Fork 1
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
rework error messages #14
Conversation
R/checks.R
Outdated
is_d_input <- function(x) { | ||
tmp <- purrr::map(x, check_numeric, input = "desirability") | ||
tmp <- purrr::map(x, check_unit_range) | ||
is_d_input <- function(x, call) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The next PR will use a constructor to check these things and give the desirability vectors classes c("numeric", "desirability")
without going full vctrs.
That will make these checks a lot easier/better. The current method would let something like roc_auc
in without an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
first round of comments! once they are resolved we can go back and see if there are more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just flagging one missed substitution.
if (!is.vector(x) || !is.numeric(x)) { | ||
rlang::abort(paste0(input, " should be a numeric vector.")) | ||
cli::cli_abort("{.arg {input}} should be {an} numeric vector.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This substitution will raise an error, as there's no an
in the fn environment.
Updates to testing with:
expect_snapshot_error()
converted toexpect_snapshot(..., error = TRUE)