Support using by instead of keyby in grouped operations #136
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Originally the first parameter was "step", but since the first parameter should be a call, it didn't make sense to have it named "step".
Removed commented code and updated translation vignette
Fix silly typo
Update NEWS.md with key parameter
Renamed link_by_struct parameter for clarity
Originally the first parameter was "step", but since the first parameter
should be a call, it didn't make sense to have it named "step".
Renamed 'with' to 'using'
with had a somewhat clear meaning, but using makes the most sense
("using by" or "using keyby").
…into feature/support-by
hadley
reviewed
Dec 10, 2019
∆ `group_by` parameter `key` is now `arrange` to follow dplyr naming conventions. ∆ Internally, `keyed` is now `keyby` to be explicit about which data.table functionality it is associated ∆ The `by_struct` system is now just a function that modifies a call. Currently accepts three variables (call, groups, and keyby), but probably could be reduced to the call and current step.
hadley
reviewed
Dec 23, 2019
∆ The `keyby` parameter in the step objects is now `arrange` for consistency ∆ `add_grouping_parameter` renamed to `add_grouping_param` and now has two arguments: a call and a step ∆ Updated translation vignette
|
Thanks! |
|
@psanker , thank you, this was a good issue to solve. The only problem left is that |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses #85
This amends how
keybyis added to the translated calls by creating a separate structure for by/keyby and a linking function to link the object to the call. It's a bit more flexible than checking if there are groups and then adding them. Users are able to accessby=by settingkey = FALSEingroup_by():I'm still not certain about the term
keyfor the parameter name since it could be confused withkey_by. Maybe it could be something likeuse_byto be explicit?