Skip to content

Conversation

mgirlich
Copy link
Contributor

@mgirlich mgirlich commented Mar 3, 2021

Fixes #1071 .

With the recent effort of bringing tidyr verbs to dtplyr I guess it makes sense to make these functions generic now.

Copy link
Member

@hadley hadley 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 — one comment about the position of ... and then can you please add a bullet to NEWS?

R/chop.R Outdated
@@ -89,7 +96,13 @@ chop <- function(data, cols) {

#' @export
#' @rdname chop
unchop <- function(data, cols, keep_empty = FALSE, ptype = NULL) {
unchop <- function(data, cols, keep_empty = FALSE, ptype = NULL, ...) {
Copy link
Member

Choose a reason for hiding this comment

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

In general, I think it's better to put the dots before the first optional argument. It's possible that a few people might have relied on partial or position based matching, but check_dots_used() will catch that.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In build_wider_spec() I put the dots after names_from and values_from as they felt like main arguments and that's also how they were used in the test names_glue affects output names

@DavisVaughan DavisVaughan added the feature a feature request or enhancement label Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

more functions should be generic
3 participants