Issue #388: Refactor preprocessing functionality

[athowes]

Description

Important

Current review ask ready for review

This PR will close #388.

Summary of what has been done:

• Add epidist_linelist class
• Add function as_epidist_linelist as date based entry point to epidist_linelist class
• Added ad-hoc function as_epidist_linelist_time as temporary time based entry point to epidist_linelist class
• Changed validation approach to use epidist_validate_data for data classes (like epidist_linelist) and epidist_validate_model for model classes (like latent_individual)

Summary of remains to do:

• Update vignettes to run
  • Ebola vignette
  • Approximate inference vignette
  • Get started vignette
  • FAQ
• Update text in vignettes
  • Ebola vignette
  • Approximate inference vignette
  • Get started vignette
  • FAQ

Summary of new issues / extensions:

1. Handling of datetimes in as_epidist_linelist #415 as_epidist_linelist could accept a user not passing an upper bound (and could assume that no upper bound means daily censoring)
2. Add aggregate data support #412 Think about other classes like epidist_linelist
3. Further entrypoints to linelist data class #414 Provide more entry pathways to epidist_linelist including a better thought out time entry point
4. Handling of case column in linelist data class #413 Right now epidist_linelist doesn't include a column for the "case" -- should we enforce that users provide this? It could be useful to have to produce plots and things
5. Refactor creating new columns in as_latent_individual #416 RIght now as_latent_individual does some making new columns. Can we refactor these into helpers as was original plan?
6. Update pass through model constructor to use epidist_linelist #402 In the class constructor for direct_model they are called ptime and stime and use a data.frame without going via epidist_linelist -- need to sort this out to go via direct_model

Checklist

• My PR is based on a package issue and I have explicitly linked it.
• I have included the target issue or issues in the PR title in the for Issue(s) issue-numbers: PR title
• I have read the contribution guidelines.
• I have tested my changes locally.
• I have added or updated unit tests where necessary.
• I have updated the documentation if required.
• My code follows the established coding standards.
• I have added a news item linked to this PR.
• I have reviewed CI checks for this PR and addressed them as far as I am able.

[athowes]

Note that actually row_id is used in the latent_indvidual model (though of course this could be created more local to the function below, which is epidist_stancode.latent_individual):

brms::stanvar(
  block = "data",
  scode = "array[N - wN] int noverlap;",
  x = filter(data, woverlap == 0)$row_id,
  name = "noverlap"
) +

[seabbs]

As it's a transformed variable I think we can create it in latent individual. As it's an internal variable we might want to use .row_id to avoid closes with user variables

[athowes]

Finding a pain point here is changing all the tests to use a date version of simulated data (knowing that probably we will need to revert this as we add functionality to have non-date data).

Makes me think of having some temporary way to class data that's already in the right format with epidist_linelist (rather than the date entry point). Can then also run it through epidist_validate_data.epidist_linelist to make sure it's OK. We will need to work out this out properly in a future issue but I think it's the right way to get things moving.

I think solution is just a function like:

as_epidist_linelist_time <- function(data) {
  class(data) <- c("epidist_linelist", class(data))
  epidist_validate_data(data)
  return(data)
}

[seabbs]

Sure but just as a helper function in test that?

[athowes]

Sure but just as a helper function in test that?

It's currently a regular packaged.

It'll need to be used in some of the vignettes as well so would prefer to keep it as a packaged function.

We could also consider putting it as an internal function and using epidist::: in the vignettes.

[athowes]

Sure but just as a helper function in test that?

Note that I think we are going to need this functionality eventually anyway, so if this is something we want to think about and implement here I could be open to it.

[seabbs]

We can't use an internal function in a vignette as it isn't CRAN complaint or good practice.

My suggestion to use as a testing function is that we can implement it here quickly and think about the design in another issue as I don't think we want to rush it.

My preference would be this is part of the simulation workflow and likely works via adding dates to simulated times but as above I think it needs some thought

[athowes]

In that case I'm in favour of adding it as an exported function (as is currently) so that it can be used in the vignettes.

Then that would give us chance to:

[implement] here quickly and think about the design in another issue as I don't think we want to rush it.

[seabbs]

This looks good.

I think I would have just a single validate function and dispatch into it from models and data. In the future we should also update to use modification validation vs entry to use validation. We should also add a new_epidist_linelist method in the future as well.