Order of input arguments to wp_request about ecmwfr HOT 5 CLOSED

fixed see issue 5

from ecmwfr.

Hi! I'm aware that this would be a breaking change but, if I may give my opinion, for me the "natural" order is to have request as the first argument. For me, the user is a mere technicality, and the important bit is what data I want to request/transfer. The user is also a parameter that probably doesn't change a lot.
Having it as the first argument would also make it pipable. So the if an user had a custom function that builds the request, they could do something like:

build_request("era4", "gepotential") %>%
   wf_request(...)

Or, with a list of requests:

walk(request_lists, wf_request)

Anyway. That's my two cents. Feel free to ignore!

from ecmwfr.

Interesting idea, would this really break that many items? I don't think I call things blindly anywhere within the package, as with above.

This being said, I'm not a big fan of the pipes and in general the tidyverse way of doing things. It is good, but mostly for people who already know what they are doing. It can be a hellhole if you don't wrap your head around a simple loop first. Just my $.02.

from ecmwfr.

Yes, it takes a while to get use to using pipes. I only use it for the most simple tasks because I get lost otherwise (and it's also harder to debug).

In any case, even with no pipes, at least in my workflow I feel that the request argument is the important one that changes more often. Maybe there are people that use many different users and feel differently. It's fairly subjective.

Changing the order of arguments would break any scripts that call wf_request() without naming arguments. It would probably be nice to have a catch like:

if (is.character(request)) {
   stop("request must be a named list. Argument order changed in version x.x.x")
}

from ecmwfr.

I'm not against pipes, but I use them sparingly in my own projects, even less or not if I can avoid it in packages. Find them hard to debug if shit hits the fan, especially unwieldy "one liners".

So sounds, good, trap the error like this and provide the option to play with pipes. More functionality I would say.

from ecmwfr.