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

Should dplyr commands gobble empty last argument? #1039

Closed
rpruim opened this issue Mar 25, 2015 · 7 comments
Closed

Should dplyr commands gobble empty last argument? #1039

rpruim opened this issue Mar 25, 2015 · 7 comments
Assignees
Labels
Milestone

Comments

@rpruim
Copy link

@rpruim rpruim commented Mar 25, 2015

It would be nice to allow a trailing comma in commands like the example below. That would make it easier to rearrange the order or to comment out lines without needing to figure out whether the commas need adjusting.

diamonds %>%
  group_by(color, clarity) %>%
  summarise(
    depth = mean(depth),
    price = mean(price),
    carat = mean(carat),  #  <-- trailing comma here currently causes errors
  )

I'm imaging a function that could be called at the top of a function definition that would gobble this empty argument. Or perhaps a wrapper function a la Vectorize() that could be used to convert any function into a function with this behavior. Seems like this could be useful for many functions both within dplyr and elsewhere.

@romainfrancois
Copy link
Member

@romainfrancois romainfrancois commented Apr 20, 2015

Hmm, I don't think I would like that. dplyr is still R code.

@rpruim
Copy link
Author

@rpruim rpruim commented Apr 20, 2015

I don't see how this would violate being R code. Is it substantially different from

x[2, ]

which is standard R usage?

Of course, you would be free not to use this feature.

@romainfrancois
Copy link
Member

@romainfrancois romainfrancois commented Apr 20, 2015

well x[2, ] is not the same as x[2]

@rpruim
Copy link
Author

@rpruim rpruim commented Apr 20, 2015

True. But it is an example where an empty argument doesn't cause things to break and is useful. So I think there is no prohibition on writing functions that allow empty arguments.

I have often wished that c() and list() had this behavior. It would make code maintenance easier.

@hadley
Copy link
Member

@hadley hadley commented Apr 20, 2015

I see this as a small violation of R's usual rules, in the service of making a common error less common. I don't see any major downsides (except that people might come to expect this in all R functions).

I remember when this wasn't an error in R, and I still miss those days :(

@romainfrancois
Copy link
Member

@romainfrancois romainfrancois commented Apr 20, 2015

In that case, we are probably looking at a change in lazy_dots or perhaps how we use it here.

@hadley hadley added this to the 0.5 milestone May 19, 2015
@hadley hadley added this to the bluesky milestone Oct 22, 2015
@hadley hadley removed this from the 0.5 milestone Oct 22, 2015
@hadley
Copy link
Member

@hadley hadley commented Feb 2, 2017

@lionel- can you take this on as part of tidyeval conversion?

@lionel- lionel- self-assigned this Feb 2, 2017
@hadley hadley added nse and removed data frame labels Feb 20, 2017
lionel- added a commit to lionel-/rlang that referenced this issue Mar 22, 2017
Can be a boolean or the string "trailing" (the default)
Fixes tidyverse/dplyr#1039
lionel- added a commit to lionel-/rlang that referenced this issue Mar 24, 2017
Can be a boolean or the string "trailing" (the default)
Fixes tidyverse/dplyr#1039
lionel- added a commit to lionel-/dplyr that referenced this issue Mar 24, 2017
@lionel- lionel- closed this in #72 Mar 24, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jun 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants