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 <epiparameter> diagram to design principles vignette #383

Merged
merged 8 commits into from
Oct 7, 2024

Conversation

joshwlambert
Copy link
Member

This PR tackles #247 by adding a mindmap of the <epiparameter> class methods. It uses {visNetwork}, which is added to Config/Needs/website in the DESCRIPTION instead of taken on a package dependency.

Hopefully this will give users an easy way to see what functionality comes with the <epiparameter> class.

Once this PR is merged I will close #247 as I believe this covers the main functions of the package, even though it is not a full architecture diagram of the package, like https://epiverse-trace.github.io/simulist/articles/design-principles.html#package-architecture. However, as {epiparameter} is made up of several disconnected functions it will be harder to provide a complete architecture diagram.

@joshwlambert joshwlambert added documentation Improvements or additions to documentation S3-method S3-class labels Sep 30, 2024
@joshwlambert
Copy link
Member Author

Annoyingly I can't remove the R CMD check note which is currently causing the CI to fail. The way to remove this would be to add {visNetwork} as a suggested package. This would add a few extra dependencies. However, this package will likely already be on users systems if they have installed {epicontacts} as it is a dependency of that package.

Overall I think this PR makes a nice addition to the package, but I want to avoid overcomplicating things and taking on too many dependencies for documentation.

Copy link
Member

@chartgerink chartgerink 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 🙌

It looks really good:

image

I had some struggle building the website, but that was because of R, not your code (I had to install libsodium headers for the sodium dependency of mrc-ide/epireview).

RE the note, this is a judgment call. If it causes all the checks from here on out to fail for epiparameter altogether, I would consider either dropping the notes from the build fail or to add visNetworks to suggests after all. It would be bad to start desensitizing (y)ourself to failing CMD checks, as that is a high priority check.

vignettes/design_principles.Rmd Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Oct 7, 2024

This pull request:

  • Adds 1 new dependencies (direct and indirect)
  • Adds 1 new system dependencies
  • Removes 0 existing dependencies (direct and indirect)
  • Removes 0 existing system dependencies

Reach out on slack (#code-review or #help channels) to double check if there are base R alternatives to the new dependencies.

(Note that results may be inaccurate if you branched from an outdated version of the target branch.)

@joshwlambert
Copy link
Member Author

This PR only changes the design-principles.Rmd vignette and the DESCRIPTION but it took several commits to get the desired end result. Therefore, I'm going to squash and merge this PR to reduce the number of commits.

Copy link

github-actions bot commented Oct 7, 2024

This pull request:

  • Adds 1 new dependencies (direct and indirect)
  • Adds 1 new system dependencies
  • Removes 0 existing dependencies (direct and indirect)
  • Removes 0 existing system dependencies

Reach out on slack (#code-review or #help channels) to double check if there are base R alternatives to the new dependencies.

(Note that results may be inaccurate if you branched from an outdated version of the target branch.)

@joshwlambert joshwlambert merged commit 7a87fc3 into main Oct 7, 2024
9 checks passed
@joshwlambert joshwlambert deleted the archi-vig branch October 7, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation S3-class S3-method
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add package architecture diagram to design principles vignette
2 participants