Closed Bisaloo closed 2 years ago
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:
regional_epinow()
, could possible hide this from user depending how much detail they want.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?
Just to check, the output of the rendered Rmd and the report generated by the Shiny app are identical with particular settings?
Yes, exactly.
Often a large print out after “Transmissibility by group”, I’m think from regional_epinow(), could possible hide this from user depending how much detail they want.
I must admit I couldn't figure out how to remove this :see_no_evil:. Do you have any idea? If not, I'll ask Sam.
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
Hi all,
one comment from my experience running the transmissibility.Rmd
file
renv::restore()
the {epiparameter}
package was still not available (fortunately, I'm a fresh user) so I though about using the {pacman}
package, replacing 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 "... number\\(Rt)"
is ... number\(Rt)
. Probably, we only need "... number (Rt)"
.
@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:
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
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.
do your prefer the review to make this edits and push commits?
The best would be to do "commit suggestions" via GitHub. This is documented in this (still in progress) chapter of the blueprints.
The ggplot2 output in y-axis of this "... number\(Rt)" is ... number(Rt). Probably, we only need "... number (Rt)"
Yes, I inherited this when I started to work on this doc. Not sure what the intention was :shrug:. 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.