Skip to content
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

kill etl_create()? #26

Open
beanumber opened this issue Jul 22, 2016 · 4 comments
Open

kill etl_create()? #26

beanumber opened this issue Jul 22, 2016 · 4 comments

Comments

@beanumber
Copy link
Owner

This is more problematic than I thought.

etl_create.default <- function(obj, ...) {
  obj <- obj %>%
    etl_init(...) %>%
    etl_update(...) %>%
    etl_cleanup(...)
  invisible(obj)
}

The problem is that these functions do not necessarily take the same arguments, so it's easy to pass unknown arguments to functions that don't take them, leading to unused argument errors.

I'm wondering if anything is lost if we simply remove etl_create. This would mean that users would have to call etl_init() explicitly if they wanted to initialize or re-initialize the database.

Note that as per #16 neither @nicholasjhorton nor @cpsievert like this function anyway.

@jsonbecker
Copy link

Have you considered using formals to filter for certain arguments?

@beanumber
Copy link
Owner Author

I haven't. How would it work here?

@jsonbecker
Copy link

If you call dots <- c(x[which(names(list(...)) %in% names(formals(etl_init)))]), for example, you could then call etl_inti(dots) and it would only contain arguments that are defined for etl_init etc.

@beanumber
Copy link
Owner Author

Right. I will think about this -- thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants