EpiNow2 icon indicating copy to clipboard operation
EpiNow2 copied to clipboard

Interface unification with `epinow2()` wrapper function?

Open hsbadr opened this issue 4 years ago • 2 comments

Both epinow() and epinow_regional() take common arguments, except for the region column in the reported_cases and the two extra epinow_regional()'s arguments (non_zero_points and summary_args) that can be ignored by epinow(). So, it's probably better to define a wrapper function for both and decide which one to call based on reported_cases$region. Here's an initial version I'm currently using:

epinow2 <- function(reported_cases, ...) {
  if (is.null(reported_cases$region)) {
    local_epinow2 <- function(reported_cases, ..., non_zero_points, summary_args) {
      epinow(reported_cases, ...)
    }
    local_epinow2(reported_cases, ...)
  } else {
    regional_epinow(reported_cases, ...)
  }
}

Note that local_epinow2 can be dropped (i.e., calling epinow() directly) if epinow accepts non_zero_points and summary_args, regardless if it'll ignore or use them.

Ideally, all documents would refer to the new epinow2() function while exporting the current functions for compatibility.

hsbadr avatar Nov 30 '20 15:11 hsbadr

I'm not totally convinced this would be that user friendly an idea? regional_epinow also accepts lists for all _opts args so that you can specify region specific settings which might get very confusing if everything was run from a single wrapper. Happy to canvas opinion on this though.

seabbs avatar Dec 02 '20 17:12 seabbs

Yes, I know that, The user can add a region column to the reported_cases and may provide region-specific options (e.g., via opts_list). The logic here is similar to providing a formula for single- vs multi-variate analysis (or single vs multi-regional epi estimates here). I think that the wrapper function will be a good addition as it won't affect the current user interface: both epinow() and regional_epinow() will be there and separate. The proposed epinow2() is just a wrapper to call the appropriate function.

hsbadr avatar Dec 02 '20 17:12 hsbadr

Converting to discussion in case anyone else would like to chime in - for now I'm considering this not a priority for implementaiotn.

sbfnk avatar Jan 29 '24 15:01 sbfnk