-
Notifications
You must be signed in to change notification settings - Fork 2
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 & EpiNow2 #31
Conversation
The updates look good. I'm not experienced with developing Shiny apps so I may have missed a couple of things. The plots and tables are generated nicely for the report. A few points on using the app:
|
Sorry, I should have been clearer that the shiny app doesn't work at this time. I have currently shelved it to focus on the Rmd file exclusively. |
No worries, will take one more look at the Rmd. Just to check, the output of the rendered Rmd and the report generated by the Shiny app are identical with particular settings? |
Yes, exactly. |
I must admit I couldn't figure out how to remove this 🙈. Do you have any idea? If not, I'll ask Sam. |
This solves the case when epiparameter_pathogen: "ebola"
Everything seems correct to me. The report is much more succinct with the output of only a single Rt analysis which I personally like. One small correction: typo on https://github.com/epiverse-trace/data_pipelines/blob/epiparameter/reports/transmissibility.Rmd#L49 |
#### Estimating $R$ from the growth rates | ||
|
||
Wallinga and Lipsitch have introduced methods for converting daily growth rates | ||
($r$) into reproduction numbers ($R$) [@Wallinga2007-hw], when information on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This citation is not rendered correctly as the bibliography is currently commented out.
Hi all, one comment from my experience running the
with if (!require("pacman")) install.packages("pacman")
pacman::p_install_gh("epiverse-trace/epiparameter") it helped me to render the Rmd, and as far as I read, it should work well with renv. footnote: For another discussion, I highly encourage to use pacman::p_load() or pacman::p_install_gh() instead of library(). It may be highly useful for novice users to avoid issues right at the beginning of their epiverse-journeys |
To improve the identification of patterns in bars plots and tables, I mean, looking first at the regions with highest incidence, we can reorder them with respect to the incidence. adding to this pipe: this total_cases <- dat %>%
select_tags(location, counts) %>%
group_by(location) %>%
summarise(cases = sum(counts)) %>%
# modification ------------- here!
arrange(location) %>%
mutate(location = fct_reorder(.f = location,
.x = cases,
.desc = F)) %>%
arrange(desc(location)) and if looking for uniformidity in this aspect, this could be also included in the Rt estimates plot and table replace all this chunk with (I only moved the filter and pipe it with the fct_reorder) res_epiestim_group_fct <- res_epiestim_group %>%
filter(end == max(end)) %>%
mutate(region = fct_reorder(.f = region,
.x = median,
.desc = F)) %>%
arrange(desc(region))
# Plot of latest estimates
res_epiestim_group_fct %>%
ggplot(aes(y = .data[[group_var]]), fill = custom_grey) +
geom_point(aes(x = median), color = dark_green) +
geom_errorbar(aes(xmin = lower, xmax = upper), color = dark_green) +
geom_vline(xintercept = 1, color = dark_pink) +
labs(title = "Estimates of Rt (EpiEstim)",
subtitle = sprintf(
"based on data from %s - %s",
format(max(get_dates(dat_i_day) - 7), "%d %B %Y"),
format(max(get_dates(dat_i_day)), "%d %B %Y")),
y = "",
x = "Instantaneous Reproduction Number (Rt)")
res_epiestim_group_fct %>%
mutate(
mean = round(mean, 2),
median = round(median, 2),
`95% ci` = sprintf(
"[%1.2f ; %1.2f]",
lower,
upper)) %>%
dplyr::select(-c(sd, lower, upper)) %>%
rename(
"mean $R$" = mean,
"median $R$" = median) %>%
set_names(toupper) %>%
kbl() %>%
kable_paper("striped", font_size = 18, full_width = FALSE) %>%
row_spec((1:n_groups)[c(FALSE, TRUE)],
background = pale_green) @Bisaloo do your prefer the review to make this edits and push commits? (I think I should've read/write the contribution guideline of the packages 😅) |
a last one is also about output aesthetics. The ggplot2 output in y-axis of this @Bisaloo let me know if this satisfy the uniformidity review that you needed. I can focus on some interpretation text during this week. With respect to the aesthetics of plots and tables, I do not have further comments. |
Thanks for your comments:
Thibaut initially used pacman but I argued for its removal because I'm not a big fan of using yet another, somewhat uncommon, tool, to sidestep something that we should take the time to explain. I'll reconsider and open an issue for input from others though.
The best would be to do "commit suggestions" via GitHub. This is documented in this (still in progress) chapter of the blueprints.
Yes, I inherited this when I started to work on this doc. Not sure what the intention was 🤷. I'll update it after merging this PR. |
Output formatting for EpiNow2 directly copied from what Thibaut wrote for EpiEstim. I'm aiming for uniformity at the moment and will probably write shared wrappers to possibly update the output later (#29).
The number of chains & samples is ridiculously low but it's to allow you to try and render the document locally in a reasonable amount of time.