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 pscl tidiers here from poissonreg #1133

Closed
wants to merge 5 commits into from
Closed

Conversation

hfrick
Copy link
Member

@hfrick hfrick commented Jan 3, 2023

part of #1132

the corresponding PR on poissonreg is tidymodels/poissonreg#58

since it's only in Suggests
@hfrick
Copy link
Member Author

hfrick commented Jan 3, 2023

The test failures look like they are unrelated to this PR 👀

Copy link
Collaborator

@simonpcouch simonpcouch left a comment

Choose a reason for hiding this comment

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

Thank you, @hfrick!

Agreed that the check failures aren't related.

While we'll plan on making some changes to these tidiers before CRAN release, this PR looks good to get that initial transfer done!

@hfrick
Copy link
Member Author

hfrick commented Jan 4, 2023

Your comment about a smooth transition on the other PR made me remember that we could make the registration of the methods here conditional on the installed version of poissonreg, i.e. if there were already registered in poissonreg (through an older version), don't try to register them again here. I'll push some changes for you to review!

@@ -551,7 +551,8 @@ Imports:
rlang,
stringr,
tibble (>= 3.0.0),
tidyr (>= 1.0.0)
tidyr (>= 1.0.0),
vctrs
Copy link
Member Author

Choose a reason for hiding this comment

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

this dependency can be removed again when we register the methods unconditionally

Comment on lines +36 to +39
tidy_zeroinfl <- .tidy

#' @rdname tidy_zip
tidy_hurdle <- .tidy
Copy link
Member Author

Choose a reason for hiding this comment

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

I've left this so that we only need to change the _ back to the . when we register unconditionally (instead of using .tidy() directly in the registration in .onLoad().

@simonpcouch
Copy link
Collaborator

Closing, with some background in this comment.🐙

@github-actions
Copy link

github-actions bot commented Feb 4, 2023

This pull request has been automatically locked. If you believe the issue addressed here persists, please file a new PR (with a reprex: https://reprex.tidyverse.org) and link to this one.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 4, 2023
@simonpcouch simonpcouch deleted the move-tidiers-poissonreg branch March 7, 2023 19:13
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.

None yet

2 participants