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

Survival summarize predictions include tuning parameters and .iter #798

Merged
merged 6 commits into from
Jan 11, 2024

Conversation

topepo
Copy link
Member

@topepo topepo commented Jan 5, 2024

These were some missing .by arguments and the columns were left out of the results.

Also reorders the columns of collect_predictions() so that starts_with(".pred") columns are to the right (previously scattered, esp in survival results).

Tests in extratests.

topepo pushed a commit to tidymodels/extratests that referenced this pull request Jan 5, 2024
@topepo topepo marked this pull request as ready for review January 5, 2024 14:02
@topepo topepo requested a review from hfrick January 5, 2024 14:02
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.

I've left a whole bunch of comments on the corresponding test PR but those largely don't seem to stem from this PR. The one I would investigate before moving further with this PR is tidymodels/extratests#158 (comment)

Additionally, I don't think this actually closes #800 , I'm gonna edit the PR description to reflect that so that we don't accidentally close it.

NEWS.md Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
Co-authored-by: Hannah Frick <hfrick@users.noreply.github.com>
@topepo topepo merged commit 09d4d08 into main Jan 11, 2024
9 checks passed
@topepo topepo deleted the survival-summarize-predictions branch January 11, 2024 02:23
topepo added a commit to tidymodels/extratests that referenced this pull request Jan 12, 2024
* initial tests

* updates for tidymodels/tune#798

* SA test updates

* update tune version

* update bayesian opt tests

* update racing unit tests

* update tune version

* temporarily point to PR

* improve readability of reference object

* racing updates; also covers changes in tidymodels/finetune#90

* updated for changes in #804

* test for #138 and tidymodels/finetune#81

* PR tune#798 has been merged, new one to check with is tune#804

* updated for new tune version

* version bumps

* update with new tune warning wording

* changes for tidymodels/tune#806

* more updated snapshots

* mroe snapshot updates

* update censored bagging snapshot

* irreproducible results; see #160

---------

Co-authored-by: ‘topepo’ <‘mxkuhn@gmail.com’>
Co-authored-by: Hannah Frick <hannah@posit.co>
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 Jan 26, 2024
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