tidyverse / magrittr

Improve the readability of R code with the pipe
https://magrittr.tidyverse.org
Other
957 stars 157 forks source link

Use () after all function calls #232

Open krlmlr opened 3 years ago

krlmlr commented 3 years ago

in README.

lionel- commented 3 years ago

Stefan prefers without parentheses.

smbache commented 3 years ago

Stefan prefers without parentheses.

Yeah, IMO that's way cleaner and less typing. Also, I think more of RHS as functions/expressions, so like in that one does not write lapply(foo, bar()). I know the analogy does not fully extend to the situation with other args, but still I prefer thinking of RHS as not being a function call standing on its own... I like the cleanliness and the interpretation that comes with no parens.. but that's just my opinion.

krlmlr commented 3 years ago

Thanks. A few thoughts:

How about a new section about omitting the parentheses for unary functions?

smbache commented 3 years ago

Well the style guide is obviously not a very stylish guide in this case ;-) as I said, my opinion is not the only one, and I don't have to make all decisions. I don't see omission of () any less consistent than omission of parens in arithmetic when they're not necessary and when precedence is clear.

krlmlr commented 3 years ago

236 is another reason to teach users the longer form first:

library(magrittr)
mtcars$cyl %>%
  forcats::as_factor
#> Error in .::forcats: unused argument (as_factor)

Created on 2020-12-05 by the reprex package (v0.3.0)

What's a good way to move forward?

luisDVA commented 2 years ago

Hi all, For teaching purposes, I'm trying to find any information or history on why/how %>% allows for function names without parenthesis on the RHS (for single argument uses). I tried to figure out if internally the pipe is using match.fun or something else but I am still unsure. I think this is relevant, given that the new base pipe does not allow functions on the RHS without parenthesis even when the only argument is whatever is being piped into them.

Thanks!

IndrajeetPatil commented 1 year ago

FWIW, not including a function call produces a lint:

library(lintr)

# will produce lints
lint(
  text = "1:3 %>% mean %>% as.character",
  linters = pipe_call_linter()
)
#> <text>:1:9: warning: [pipe_call_linter] Use explicit calls in magrittr pipes, i.e., `a %>% foo` should be `a %>% foo()`.
#> 1:3 %>% mean %>% as.character
#>         ^~~~
#> <text>:1:18: warning: [pipe_call_linter] Use explicit calls in magrittr pipes, i.e., `a %>% foo` should be `a %>% foo()`.
#> 1:3 %>% mean %>% as.character
#>                  ^~~~~~~~~~~~

# okay
lint(
  text = "1:3 %>% mean() %>% as.character()",
  linters = pipe_call_linter()
)

Created on 2022-10-11 with reprex v2.0.2

Additionally, native pipe requires a function call:

library(lintr)
lint(
  text = "1:3 |> mean |> as.character",
  linters = pipe_call_linter()
)
#> <text>:1:1: error: [error] The pipe operator requires a function call as RHS
#> 
#> ^

Created on 2022-10-11 with reprex v2.0.2

For these reasons, I agree with Kirill here that the README should include function calls.