[FEATURE] Reduce osmdata dependencies: magritrr

[jmaspons]

The base pipe operator |> is available since R 4.1 and can substitute magrittr::%>%(). The use of %>% suppose a tiny overhead but can be avoided internally without affecting the API. From the user perspective, removing magrittr from imports can break some code because it would require explicitly loading the package if the user use %>% and some examples in osmdata would need updates.

We have different options with different:

• Require R >= 4.1 and use |> instead of %>%.
• Avoid |> and %>% in the internal code.
  • %>% can be keep it exported. magrittr can be removed from dependencies in the future without currently impacting the users.
  • Rewrite the examples and vignettes without pipes. Remove magrittr from the dependencies and remove %>% from exported functions.

What do you think, @mpadge ?

[mpadge]

Thanks @jmaspons, this is definitely worth thinking about. I do think that the general pipey-workflows demonstrated through the vignettes are likely the way most people will use this package, and so should be kept, rather than re-writing in non-piped forms. BUT almost all of those chunks are not executed anyway. And that means we could probably just remove magrittr, and yet still keep current R (>= 3.2.4). That would be great

[mpadge]

Shall I tackle this one, or would you like to give it a go?

[jmaspons]

I can try to prepare a PR. I would go for a non-pipe rewrite in the function's code to avoid increase R version dependence and rewrite the examples and vignettes with the base pipe (|>). Most of the examples are enclosed in dontrun environments and should pass the checks but would fail for users if magrittr is not loaded or R < 4.1 (hopefully a minority nowadays).

[jmaspons]

I have a branch at https://github.com/jmaspons/osmdata/tree/no_magrittr, but I want to test it extensively before a merge to avoid surprises on release time (devtools::check_win_release(), devtools::check(remote = TRUE, manual = TRUE), et al.)

[jmaspons]

I send some tests. Could you approve them and post the link with the results, @mpadge ?

Wait. I get timeouts...

[mpadge]

Do you want to just PR your branch to here?

[mpadge]

@jmaspons Thanks so much for all the work in #361 - I really appreciate it. I'm going to re-open this to tidy a few final things for CRAN re-submission. I suspect they might note that we're using native pipes without R >= 4.1, and then notice that too many examples are entirely wrapped in \dontrun (of actually if (FALSE) {...}). We can and should improve examples to ensure as much as possible is actually run, and then update https://github.com/ropensci/osmdata/blob/main/cran-comments.md to explain everything.

[jmaspons]

I would go with

#' @examplesIf getRversion() >= "4.1"
#' ...
#' @examplesIf getRversion() < "4.1"
#' ...

I can prepare a PR if you want, @mpadge

[mpadge]

It's a good idea, but we can't unfortunately because all the examples with pipes pass place names to opq, which then calls the Nominatim server, meaning those can't be run anyway,

[jmaspons]

The idea is to show examples with only the code that the user can run, regardless of the \donrun. So users with R < 4.1 shouldn't see any pipe. Example at https://github.com/jmaspons/osmdata/commit/58e270004adde55c02de015bc75da36c3ca9d681