epiverse-trace / epiparameter

R package with library of epidemiological parameters for infectious diseases and functions and classes for working with parameters
https://epiverse-trace.github.io/epiparameter
Other
33 stars 11 forks source link

Unify S3 class design principles with {contactmatrix} #364

Open joshwlambert opened 3 months ago

joshwlambert commented 3 months ago

The {contactmatrix} R package aims to unify the underlying data structure used by packages that use contact matrices. {epiparameter} aims to do some very similar with its <epiparameter> class (previously <epidist>). Therefore, as these two projects have overlap in their development teams, it makes sense to align their design principles as much as possible.

This issue will track those developments.

Tagging @Bisaloo.

joshwlambert commented 3 months ago

Question on class standards: When should a element of a class be required versus optional?

In {epiparameter} this relates to the list elements of an <epiparameter> object. Currently all must be present for the object to be considered valid. But it may be the case that the less important elements (e.g. $notes) could be removed and the object is still considered valid.

More generally this question could be applied to a class based on a list, or another non-atomic object, or an atomic object using attributes.

joshwlambert commented 4 days ago

From the recent {contactmatrix} dev day I become a bit more familiar with the codebase. The two packages are well aligned from a design perspective. One remaining minor difference is the class name for a dedicated class for a list of objects, <multi_epiparameter> and <contactmatrix_list>.

Is it worth renaming one of these so they are consistent, e.g. <epiparameter_list> or <multi_contactmatrix>. This would not have any functional different.

@Bisaloo what do you think?

Bisaloo commented 1 day ago

It makes sense from a conceptual point of view but I'd only do it if it's an easy lift.

I'm still slightly unsure which class name makes more sense :thinking:

(Note that this misses cases where class is not set by class<-())