gesistsa
Using an existing function for filtering `...` instead of writing it out each time.
I'm watching a series of related issues and PRs here re: ....
The tidyverse/r-lib packages are using the ellipsis package for similar functionality, in case that holds any interest. Yes, it would be a dependency. But it would also eliminate the need to grow your own solution here. YMMV 🤷♀️
Originally posted by @jennybc in https://github.com/leeper/rio/pull/225#issuecomment-534315342
I'm watching a series of related issues and PRs here re:
....The tidyverse/r-lib packages are using the ellipsis package for similar functionality, in case that holds any interest. Yes, it would be a dependency. But it would also eliminate the need to grow your own solution here. YMMV woman_shrugging
Thank you, this is a helpful package. I don't see a way to have it actually filter out unused arguments in order to prevent an error, though. This got me thinking, though-- you're right, we can't be the first people encountering the problem of ... being passed to a function which doesn't have ... as a formal argument. In fact, Hadley Wickham had this question back in 2008 and he didn't want to reinvent the wheel by implementing it himself. After some more digging I found R.utils::doCall which does exactly what we need except that it drops argument silently.
I submitted an issue to that repo and if feedback is favorable I will submit a pull request to them which implements warnings as an option.
As you kind of alluded, we don't know how @leeper will feel about adding another dependency just for this. But if so, many of the .import.rio_* methods could follow a very consistent pattern...
Instead of
.import.rio_ods <- function(file, which = 1, header = TRUE, ...) {
requireNamespace("readODS")
readODS::read_ods(path = file, sheet = which, col_names = header, ...)
}
we would have this...
.import.rio_ods <- function(file, which = 1, header = TRUE, ...) {
requireNamespace("readODS")
a <- list(...)
a[c('path', 'sheet', 'col_names')] <- c(file, which, header)
doCall(readODS::read_ods, a)
}
...and for many cases the only things different from one underlying import function to another are the names of values in a that get created/overwritten and the function object.
Thank you, this is a helpful package. I don't see a way to have it actually filter out unused arguments in order to prevent an error, though.
I think the ellipsis mentality is that we usually want to error for unused arguments. If you provided foreign arguments to a function that did not have ..., you'd get an error. So often a user would like to know that some of their arguments are being quietly sent to /dev/null.
However, sometimes this is necessary. That recently came up in devtools (https://www.tidyverse.org/articles/2019/09/devtools-2-2-1/#ellipsis), which lead to more flexibility being built into ellipsis (https://github.com/r-lib/ellipsis/commit/dc23a8c7b8cf644157dad5e67d078ee9d4ac64b6).
So this is related to #132. Basically, we need:
- a schema that provides "standard" argument names that we will use for common behavior
- something like the crosswalk you created @bokov that maps function/package argument naming schemes to the rio standard
- a function that takes argument lists and translates them from rio standard to package standard where appropriate, passing forward anything else, and erroring on invalid arguments
I think there's a slight annoyance that we can't use do.call() with suggested packages as the function name isn't found, ex:
> requireNamespace("data.table")
Loading required namespace: data.table
> do.call("fread", list("foo.csv"))
Error in fread("foo.csv") : could not find function "fread"
> do.call("data.table::fread", list("foo.csv"))
Error in `data.table::fread`("foo.csv") :
could not find function "data.table::fread"
Actually, it looks like that might be solvable with:
do.call(getFromNamespace("fread", "data.table"), list("foo.csv"))
https://r.789695.n4.nabble.com/do-call-method-within-namespace-td797206.html
So this is related to #132.
Yes, and also #245. Actually have some code in a branch for that.
Actually, it looks like that might be solvable with:
do.call(getFromNamespace("fread", "data.table"), list("foo.csv"))https://r.789695.n4.nabble.com/do-call-method-within-namespace-td797206.html
Cooool! I learned something new today, thank you.