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

Move .censoring weights graf.workflow #1038

Merged
merged 3 commits into from
Dec 13, 2023

Conversation

EmilHvitfeldt
Copy link
Member

@EmilHvitfeldt EmilHvitfeldt commented Dec 12, 2023

to close #968

Paired {workflows} PR: tidymodels/workflows#213

There are 4 possible scenarios:

.censoring_weights_graf.workflow() is exported from only {parsnip}

This is fine!

Happens if parsnip < 1.1.1.9007 and workflows < 1.1.3.9001, which is another way of saying that both packages are installed before Dec 12.

.censoring_weights_graf.workflow() is exported from only {workflows}

This is fine!

Happens if parsnip >= 1.1.1.9007 and workflows >= 1.1.3.9001, which is another way of saying that both packages are installed after Dec 12 (or whenever the PRs are merged in.

.censoring_weights_graf.workflow() is exported from both {parsnip} and {workflows}

This is not good.

Would need parsnip < 1.1.1.9007 and workflows >= 1.1.3.9001. Which can't happen as workflows 1.1.3.9001 requires parsnip >= 1.1.1.9007. So we are good here.

##.censoring_weights_graf.workflow() isn't exported

Would happen if parsnip >= 1.1.1.9007 and workflow < 1.1.3.9001. Which could happen. I can't recall if there is a way for us to deal with this last point

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! We have previously done a more complicated approach with conditionally registering the moved method but since this method has not gone to CRAN yet, I think the approach via requiring a fixed version of parsnip is sufficient here.

@EmilHvitfeldt EmilHvitfeldt merged commit 8f13c1c into main Dec 13, 2023
7 checks passed
@EmilHvitfeldt EmilHvitfeldt deleted the move-.censoring_weights_graf.workflow branch December 13, 2023 20:27
hfrick added a commit to tidymodels/extratests that referenced this pull request Dec 15, 2023
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 28, 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.

Move .censoring_weights_graf.workflow() to workflows
2 participants