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

Add design principles vignette #68

Merged
merged 5 commits into from
Dec 14, 2023
Merged

Add design principles vignette #68

merged 5 commits into from
Dec 14, 2023

Conversation

joshwlambert
Copy link
Member

This PR addresses #28 by adding a design principles vignette. This vignette outlines the scope of the package, the function signatures and naming convention, as well as the function return types, it covers development design decisions and some explanation to package dependencies.

@joshwlambert joshwlambert added documentation Improvements or additions to documentation design Updates related to the design of the package labels Dec 12, 2023
Copy link

@sbfnk sbfnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @joshwlambert. This looks great.

Is it worth at this point reconsidering whether it would make merge this and <epichains> into a single package that revolves around analysis and interpretation of infectious disease data with branching process models? The main reason I'm thinking that this could make sense is that the two have somewhat circular dependencies: the mixture probability distributions implemented here would be useful in <epichains> (which already implements a gamma/Borel mixture) as would the functions for providing secondary analysis given parameters of a negative binomial - whereas this package itself depends on <epichains>/<bpmodels> for simulation.

If I remember correctly we previously decided to keep them separate as <bpmodels> was transitioning into <epichains> when this package came into existence but as this transition is mostly complete it might be a good time to come back to this.

cc @jamesmbaazam and @adamkucharski for thoughts.

@adamkucharski
Copy link
Member

Agree makes sense to reconsider whether better as single package as a 'one stop shop' for branching analysis – currently superspreading is more focused on offspring distributions and analytical solutions vs branching simulation in epichains, but as you say @sbfnk there is potential for benefits of shared functionality across several aspects.

@sbfnk
Copy link

sbfnk commented Dec 14, 2023

Great. Just to note that I've edited my initial comment having realised that none of the package names had been rendered - hopefully makes a bit more sense now.

@joshwlambert
Copy link
Member Author

Thanks for the comments. I'm open to the merger. I think for now it would benefit both packages to reach stability and then be merged. Therefore, I propose we wait until after {epichains} has been reviewed and released and another {superspreading} release, and then we can coordinate a possible merge.

Copy link
Member

@jamesmbaazam jamesmbaazam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Josh. I like that the document is very clear about the scope of the package. I've always wondered if this document is also a good place to compare other related packages.

@jamesmbaazam
Copy link
Member

Thanks for the comments. I'm open to the merger. I think for now it would benefit both packages to reach stability and then be merged. Therefore, I propose we wait until after {epichains} has been reviewed and released and another {superspreading} release, and then we can coordinate a possible merge.

Thanks for your inputs, all. I do think that merging the two packages would be a good idea because they have enough overlap. Additionally, a single package would be easier to maintain since any breaking changes in {epichains} would affect {superspreading}. If the agreement is to merge it, I would suggest holding off another release of {superspreading} and rather merging it into {epichains} before the v0.1.0 release in the new year. We can then jointly supersede {bpmodels} and {superspreading}.

@joshwlambert
Copy link
Member Author

Thanks for all the input. I will merge this PR once all checks pass. I think the discusson on merging should be continued elsewhere.

@joshwlambert joshwlambert merged commit 57d1bc0 into main Dec 14, 2023
8 checks passed
@joshwlambert joshwlambert deleted the add_design_vig branch December 14, 2023 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Updates related to the design of the package documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants