search

sfirke / janitor

simple tools for data cleaning in R
http://sfirke.github.io/janitor/
Other
1.38k stars 132 forks source link

Remove `%>%` in favor of `|>`? #562

Closed billdenney closed 8 months ago

billdenney commented 10 months ago

We currently export %>%.

The built-in |> was added in R 4.1 (about 2 years ago). I think that it would make sense to remove the magrittr pipe in favor of the built-in pipe throughout the package.

sfirke commented 8 months ago

I was just thinking this myself as I reviewed #559 . I agree it's reasonable to stop using %>% in the code and to stop exporting it.

olivroy commented 8 months ago

The only thing is that the tidyverse has the policy of supporting the last 5 versions of R. I know that Hmisc depending on R 4.1 for this exact workflow broke CI for packages like ggplot2 on R 4.0 for example.

I know that purrr uses |> in examples. https://github.com/tidyverse/purrr/pull/938 and has a hack for avoiding builds to work on 4.0 and 3.6.

I think it would be a good idea to stop re-exporting it as a first step? That would be a breaking change technically, but I am not sure many people rely on janitor to get the magrittr pipe in the first place.

sfirke commented 8 months ago

The stop re-exporting it seems like a good first step, it would be the breaking part for users. We should bump the janitor version number to indicate that. I think the next release could be 3.0.

olivroy commented 8 months ago

If the pipe is not re-exported, the examples using %>% will not work without an explicit library(magrittr) call. I may look into implementing the workaround used in tidyverse packages https://fosstodon.org/@gaborcsardi/111821576530612948

billdenney commented 8 months ago

Thanks for looking that up.

To me, that seems like a lot of effort for a relatively small gain. When R 4.0 is very old (for some now-uncertain definition of "very"), I think that we should switch to |> when the effort is near-zero.

I will close this for now.