-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Using an existing function for filtering ...
instead of writing it out each time.
#231
Comments
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 |
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). |
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" |
Actually, it looks like that might be solvable with:
https://r.789695.n4.nabble.com/do-call-method-within-namespace-td797206.html |
Cooool! I learned something new today, thank you. |
So did I! |
Might this also be closed by #251 ? |
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)
The text was updated successfully, but these errors were encountered: