Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upUsing an existing function for filtering `...` instead of writing it out each time. #231
Comments
This comment has been minimized.
This comment has been minimized.
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 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 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 |
This comment has been minimized.
This comment has been minimized.
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 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 (r-lib/ellipsis@dc23a8c). |
This comment has been minimized.
This comment has been minimized.
So this is related to #132. Basically, we need:
I think there's a slight annoyance that we can't use > 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" |
This comment has been minimized.
This comment has been minimized.
Actually, it looks like that might be solvable with:
https://r.789695.n4.nabble.com/do-call-method-within-namespace-td797206.html |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Cooool! I learned something new today, thank you. |
This comment has been minimized.
This comment has been minimized.
So did I! |
bokov commentedSep 25, 2019
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 #225 (comment)