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 Getting Started section on plotting with {epicontacts} #256

Merged
merged 9 commits into from
May 23, 2024

Conversation

jamesmbaazam
Copy link
Member

@jamesmbaazam jamesmbaazam commented May 21, 2024

This PR removes the plotting of aggregated cases section in the README and signposts to a new section in the Getting Started vignette on plotting individual chains with {epicontacts} and aggregated data with base R's plot function.

This PR is inspired by #255 but does not resolve it.

This PR contains unrelated .Rd files that were generated as a result of the renamed scripts in #253.

@jamesmbaazam jamesmbaazam added the documentation Improvements or additions to documentation label May 21, 2024
@jamesmbaazam jamesmbaazam added this to the v0.1.0 milestone May 21, 2024
@jamesmbaazam jamesmbaazam reopened this May 21, 2024
Copy link

This pull request:

  • Adds 7 new dependencies (direct and indirect)
  • Adds 2 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.)

@jamesmbaazam
Copy link
Member Author

This pull request:

  • Adds 7 new dependencies (direct and indirect)
  • Adds 2 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.)

{epicontacts} is a well-maintained and widely used package for plotting transmission chains, so it was a necessary dependency to be taken on.

Copy link

This pull request:

  • Adds 7 new dependencies (direct and indirect)
  • Adds 2 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.)

1 similar comment
Copy link

This pull request:

  • Adds 7 new dependencies (direct and indirect)
  • Adds 2 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.)

@jamesmbaazam
Copy link
Member Author

jamesmbaazam commented May 21, 2024

@joshwlambert The {epicontacts} plot is not rendering here using pkgdown::build_site() even though it works in my RStudio viewer when the vignette is rendered locally. Could you help me diagnose what might be wrong?

Moved this comment to the appropriate code block.

Comment on lines +509 to +520
```{r plot_chain}
# Create an `<epicontacts>` object
epc <- make_epicontacts(
linelist = longest_chain,
contacts = longest_chain,
id = "infectee",
from = "infector",
to = "infectee",
directed = TRUE
)

# Plot the chain
plot(epc)
```
Copy link
Member Author

@jamesmbaazam jamesmbaazam May 21, 2024

Choose a reason for hiding this comment

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

@joshwlambert The {epicontacts} plot is not rendering here using pkgdown::build_site() even though it works in my RStudio viewer when the vignette is rendered locally. Could you help me diagnose what might be wrong?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think the issue is with the code block highlighted here, but rather with the vignette yaml header.

I experienced similar issues getting the interactive {epicontacts} plotting to work in {simulist}. In the end, the solution that I chose was to use rmarkdown::html_vignette as the output rendering. https://github.com/epiverse-trace/simulist/pull/22/files#diff-6c0ddf0e8bb8afdcb174c0f67c09aabd60c69f62f0fa169d889119fe5600bef0

This however has some downsides. See this bullet point in the Design principles vignette in {simulist} where I explain what the limitations are and why I chose the options I did in order to get the interactive plotting to work correctly.

It's a tradeoff, I preferred the interactive plotting over the other vignette features for {simulist}, but you may decide differently depending on the importance of this code chunk.

If this info does not solve your issue let me know and I'd be happy to take another look.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, Josh. It was indeed the output format. This worked for me. I'll assess the drawbacks and see if it's worth it.

Copy link

This pull request:

  • Adds 7 new dependencies (direct and indirect)
  • Adds 2 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.)

Copy link

This pull request:

  • Adds 7 new dependencies (direct and indirect)
  • Adds 2 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.)

@codecov-commenter
Copy link

codecov-commenter commented May 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.03%. Comparing base (514feab) to head (448d4a1).
Report is 128 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #256   +/-   ##
=======================================
  Coverage   99.03%   99.03%           
=======================================
  Files           8        8           
  Lines         729      729           
=======================================
  Hits          722      722           
  Misses          7        7           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jamesmbaazam jamesmbaazam merged commit a7faf3a into main May 23, 2024
10 checks passed
@jamesmbaazam jamesmbaazam deleted the add-chain-plot branch May 23, 2024 15:31
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
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants