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

support eval_time in fit_best() and rank_results() #144

Merged
merged 7 commits into from
Feb 28, 2024
Merged

Conversation

simonpcouch
Copy link
Contributor

@simonpcouch simonpcouch commented Feb 21, 2024

An initial, incomplete go at introducing full support for survival in workflowsets. Still some more work to do and tests to add, though my COVID brain is tired, so I'm going to go nap.😴

fit_best.workflow_set() handles eval_time as much like fit_best.tune_results() as it can, and the same goes for rank_results() and select_best.tune_results().

Note that this PR will introduce a dev tune dependency. Related to #142.

@simonpcouch simonpcouch linked an issue Feb 21, 2024 that may be closed by this pull request
`fit_best.workflow_set()` handles `eval_time` as much like `fit_best.tune_results()` as it can, and the same goes for `rank_results()` and `select_best.tune_results()`
simonpcouch added a commit to tidymodels/extratests that referenced this pull request Feb 22, 2024
@simonpcouch simonpcouch marked this pull request as ready for review February 22, 2024 15:13
@simonpcouch simonpcouch requested a review from hfrick February 22, 2024 15:31
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.

Looks good! In light of the recent discussion, I'd change the argument order for rank_results() but I think you anticipated that already :)

R/rank_results.R Outdated Show resolved Hide resolved
@simonpcouch simonpcouch merged commit 34b673f into main Feb 28, 2024
10 checks passed
@simonpcouch simonpcouch deleted the survival-143 branch February 28, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fit_best.workflow_set needs eval_time argument
2 participants