-
Notifications
You must be signed in to change notification settings - Fork 420
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
Make uncount()
generic
#1358
Make uncount()
generic
#1358
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I am fine with making uncount()
generic, as it does seem more stable than the other functions mentioned in #1071, as you said. @hadley should weigh in about this though.
I agree with the placement of the dots. I am running revdeps now and will report back with the results. Hopefully no one was relying on positional args for .remove
or .id
here.
Could you also please add two NEWS bullets? One that mentions that ...
have been added between the required and optional arguments, and one that mentions that uncount()
has become generic. Both should reference this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me.
Revdepcheck shows no problems from this PR! |
Thanks @mgirlich ! |
I guess that
uncount()
is more stable than the other functions mentioned in #1071. This would currently be useful for dtplyr tidyverse/dtplyr#352@DavisVaughan Are you fine with making
uncount()
generic or do you think the interface is not yet stable (enough)?