Skip to content

survival time to binary conversion #973

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 12 commits into from
Jun 8, 2023
Merged

survival time to binary conversion #973

merged 12 commits into from
Jun 8, 2023

Conversation

topepo
Copy link
Member

@topepo topepo commented May 19, 2023

This exports three functions in the standalone file (one function is new). It exports them in a separate file that uses some razzle-dazzle from the sources for with::defer.

Unit tests are here; no more needed in extratests for the new function.

@topepo topepo requested a review from hfrick May 19, 2023 15:51
@hfrick
Copy link
Member

hfrick commented Jun 7, 2023

I've updated the documentation to match the language in the current draft for the tidymodels.org articles. About the unit tests: I've moved them to a file name so that we have that correspondence to the R file name. It's neat to get around the dependency on the survival package for the test by just defining the structure but all the other functions in that file are tested in extratests already. Are you up for me moving the test to extratests so that we only have one place to find those tests?

@topepo
Copy link
Member Author

topepo commented Jun 8, 2023

It's neat to get around the dependency on the survival package for the test by just defining the structure but all the other functions in that file are tested in extratests already. Are you up for me moving the test to extratests so that we only have one place to find those tests?

Sure, that would be good

@topepo topepo marked this pull request as ready for review June 8, 2023 13:10
@hfrick
Copy link
Member

hfrick commented Jun 8, 2023

Cool, I'll do that and solve the merge conflict in NEWS. Once that's done, I'll merge this PR 👍

@hfrick hfrick merged commit a85e508 into main Jun 8, 2023
@hfrick hfrick deleted the time-to-binary branch June 8, 2023 14:45
@github-actions
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 Jun 23, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants