beanumber / etl

R package to facilitate ETL operations
126 stars 21 forks source link

kill etl_create()? #26

Open beanumber opened 7 years ago

beanumber commented 7 years ago

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 commented 7 years ago

Have you considered using formals to filter for certain arguments?

beanumber commented 7 years ago

I haven't. How would it work here?

jsonbecker commented 7 years ago

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 commented 7 years ago

Right. I will think about this -- thanks!