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

Unify S3 class design principles with {contactmatrix} #364

Open
joshwlambert opened this issue Aug 12, 2024 · 3 comments
Open

Unify S3 class design principles with {contactmatrix} #364

joshwlambert opened this issue Aug 12, 2024 · 3 comments
Labels

Comments

@joshwlambert
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Member

Bisaloo commented Nov 30, 2024

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 🤔

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

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

No branches or pull requests

2 participants