-
Notifications
You must be signed in to change notification settings - Fork 42
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
conversion of warnings and errors to cli #954
Conversation
@@ -372,37 +364,37 @@ check_metrics <- function(x, object) { | |||
is_surv_metric_set <- inherits(x, c("survival_metric_set")) | |||
|
|||
if (!is_numeric_metric_set && !is_class_prob_metric_set && !is_surv_metric_set) { | |||
rlang::abort("The `metrics` argument should be the results of [yardstick::metric_set()].") | |||
cli::cli_abort("The {.arg metrics} argument should be the results of | |||
{.fn yardstick::metric_set}.") |
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.
Previous messages has [pkg:fun()]
format. Do we want to move to that? I don't think that they were clickable.
"The first argument needs to be an object with class 'tune_results'." | ||
) | ||
cli::cli_abort("The first argument needs to be {.cls tune_results} object, | ||
not {.obj_type_friendly {mtcars}}.") |
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.
I'm using obj_type_friendly
a lot in these messages.
cli::cli_abort( | ||
"There is no `fit_best()` method for an object with \\ | ||
{cli::qty(cls)} class{?es} {.var {cls}}." | ||
"No {.fn fit_best} exists for {.obj_type_friendly {x}}." |
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.
I standardized on this message for all of the main default methods.
@@ -322,7 +320,8 @@ plot_param_vs_iter <- function(x, call = rlang::caller_env()) { | |||
param_cols <- get_param_columns(x) | |||
pset <- get_param_object(x) | |||
if (is.null(pset)) { | |||
rlang::abort("`autoplot()` requires objects made with tune version 0.1.0 or later.") | |||
cli::cli_abort("{.fn autoplot} requires objects made with {.pkg tune} |
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.
Maybe time to retire this?
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.
Agreed.
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.
I feel like this PR is going to make tidymodels feel much more modern/slick for many users. Heck yeah!
Co-authored-by: Simon P. Couch <simonpatrickcouch@gmail.com>
Co-authored-by: Simon P. Couch <simonpatrickcouch@gmail.com>
…nto cli-conversion
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue. |
pal was a godsend here 💯