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

Refactored functions to select single evaluation times #778

Merged
merged 32 commits into from
Dec 6, 2023

Conversation

topepo
Copy link
Member

@topepo topepo commented Dec 5, 2023

Primarily exports choose_eval_time() for use in select/show best functions.

More changes will be coming for autoplot() and similar.

Places where select_best() is used: augment.tune_results(), fit_best(). These will need more work since they do not have eval_time arguments.

There is also still some legacy code that overlaps with this and previous changes (e.g. middle_eval_time()). These will be removed in a subsequent PR.

  1. Tools for selecting a default evaluation time #768
  2. Refactor choose_metric #777
  3. (you are here) functions to validate the chosen evaluation time with updates to show_best() and select_best() (see When we need to default to a single value for eval_time #766).
  4. update finetune to use the change
  5. add additional functions that return (potentially) multiple evaluation times (eg autoplot())
  6. refactor metric input checking code in the tune_* functions, fit_resamples(). etc.
  7. write an article on tidymodels.org about this.

R/select_best.R Show resolved Hide resolved
@topepo topepo marked this pull request as ready for review December 5, 2023 22:51
@topepo topepo requested a review from hfrick December 5, 2023 22:51
Copy link
Member

@hfrick hfrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚂

R/metric-selection.R Outdated Show resolved Hide resolved
R/metric-selection.R Outdated Show resolved Hide resolved
R/metric-selection.R Outdated Show resolved Hide resolved
inst/test_objects.Rout Show resolved Hide resolved
tests/testthat/_snaps/select_best.md Outdated Show resolved Hide resolved
R/select_best.R Outdated Show resolved Hide resolved
R/select_best.R Outdated Show resolved Hide resolved
R/select_best.R Outdated Show resolved Hide resolved
R/metric-selection.R Outdated Show resolved Hide resolved
R/metric-selection.R Outdated Show resolved Hide resolved
@topepo topepo merged commit 8f99330 into main Dec 6, 2023
9 checks passed
@topepo topepo deleted the new-time-selections branch December 6, 2023 19:32
Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants