tidymodels / yardstick

Tidy methods for measuring model performance
https://yardstick.tidymodels.org/
Other
367 stars 54 forks source link

Consider soft deprecating the `yardstick.event_first` option #163

Closed DavisVaughan closed 4 years ago

DavisVaughan commented 4 years ago

This is a global option that changes computation. This is generally a bad thing, so I'd much rather it be an argument to the metric function. Possibly we could push it into the estimator argument with "binary" continuing to do use the 1st level as the event and "binary-last" using the 2nd level.

This makes sense since it is ignored anyways for macro/micro multiclass settings.

This would help with #160 since we could then provide that option to conf_mat() and pass it through to all the metrics that summary.conf_mat() computes. Or make it an option to summary.conf_mat()

We could warn if the user had set the option, but use it anyways and translate it to the estimator equivalent. This would be somewhat backwards compatible

DavisVaughan commented 4 years ago

Might also have helped with https://github.com/tidymodels/tune/issues/240. Nevertheless, we also need to think about how to pass the option through to tune

github-actions[bot] commented 3 years ago

This issue 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.