tidymodels / parsnip

A tidy unified interface to models
https://parsnip.tidymodels.org
Other
564 stars 78 forks source link

switch to {cli} in check_args() functions #1093

Closed EmilHvitfeldt closed 2 months ago

EmilHvitfeldt commented 3 months ago

Ref: https://github.com/tidymodels/parsnip/issues/1081

Now the call are being passed around, such as it is being emitted from fit() which feels appropriate. I wonder is there is a way to signal the model as well? so instead of saying Error in fit(): it says something like Error in fit() for discrim_linear(): or something to that liking. It would be nice for workflowsets

Also, I didn't detect any tests 😬 so I'll add those as well

library(parsnip)
library(discrim)

discrim_linear(penalty = -2) |>
  fit(Species ~ ., data = iris)
#> Error in `fit()`:
#> ! The amount of regularization, `penalty`, should be `>= 0`.

Extension packages:

Empty tests

Some of the tests are left intentionally empty, this is either because there is a placeholder check_args() method with no checking, or no method at all. All missing methods will be handled by https://github.com/tidymodels/parsnip/issues/1094

EmilHvitfeldt commented 3 months ago

This PR converts existing checks to {cli}, expanded checking will be tracked with https://github.com/tidymodels/parsnip/issues/1095

EmilHvitfeldt commented 3 months ago

Todo: make sure we allow zero length input in input checking as per https://github.com/tidymodels/parsnip/issues/1099

github-actions[bot] commented 2 months ago

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.