add options for limiting/expanding plot limits

[sbfnk]

This is a somewhat clunky proposal to add the ability to zoom in (and limit to 0) to plots with balooning predictive uncertainty, such as in https://covid19forecasthub.eu/reports/ensemble-report-2021-03-29.html

[codecov[bot]]

Codecov Report

Merging #107 (2fa1e2e) into master (195c423) will decrease coverage by 7.69%. The diff coverage is 0.00%.

:exclamation: Current head 2fa1e2e differs from pull request most recent head 5d261fc. Consider uploading reports for the commit 5d261fc to get more accurate results

@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
- Coverage   54.56%   46.86%   -7.70%     
==========================================
  Files          19       19              
  Lines        1468     1389      -79     
==========================================
- Hits          801      651     -150     
- Misses        667      738      +71     

Impacted Files	Coverage Δ
R/plot.R	0.00% <0.00%> (-20.30%)	:arrow_down:
R/utils_plotting.R	0.00% <0.00%> (ø)
R/utils.R	63.49% <0.00%> (-7.94%)	:arrow_down:
R/utils_data_handling.R	72.64% <0.00%> (-6.61%)	:arrow_down:
R/eval_forecasts.R	66.07% <0.00%> (-1.21%)	:arrow_down:
R/pairwise-comparisons.R	42.14% <0.00%> (-0.23%)	:arrow_down:
R/pit.R	56.09% <0.00%> (ø)
R/bias.R	87.93% <0.00%> (ø)
R/interval_score.R	95.45% <0.00%> (ø)
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 195c423...5d261fc. Read the comment docs.

[Bisaloo]

@sbfnk, could you rebase this on top on the current master branch please? So we can benefit from the latest improvements when installing from this branch in reports-ensemble.yml.

If that's helpful, I can submit a PR to set up https://github.com/marketplace/actions/automatic-rebase, so we just have to comment /rebase to automatically rebase PRs.

[sbfnk]

Done - sounds good re automatic rebase if that's a straightforward thing to do.

[Bisaloo]

/rebase

[Bisaloo]

Could these functions help solve the problem more easily?

https://scales.r-lib.org/reference/oob.html

They are designed to work with ggplot2.

[sbfnk]

Could these functions help solve the problem more easily?

https://scales.r-lib.org/reference/oob.html

I don't think so - these replace data outside of plot boundaries with NA or fixed values, but we want these data to be there (so the forecast cones go in the correct direction) but cut off from the plots.

There might be something we could do with linearly interpolating between the last visible data point and the first outside the plot boundaries, but I don't think it's straightforward.

[sbfnk]

This all looks like it can be merged in to me though it would be nice if the new plotting code was in an internal function vs in the same function as there is already a lot going on + some basic unit testing.

The trouble is that it includes code copied from the ggplot2 source so will probably break in unpredictable ways when that changes. Maybe that's OK if we add a unit test that would catch the worst problems, but it's not particularly satisfactory.

[sbfnk]

/rebase

[sbfnk]

/rebase

[Bisaloo]

Note that this doesn't work well when truth data is missing. E.g., for hospitalizations in the ensemble:

[Bisaloo]

/rebase

[Bisaloo]

I managed to get the desired behaviour without having to rely on ggplot internals

library(covidHubUtils)
library(tidyverse)

raw_truth <- load_truth(
  truth_source = "JHU",
  temporal_resolution = "weekly",
  truth_end_date = "2022-02-01",
  hub = "ECDC"
)
truth <- raw_truth %>%
  mutate(model = NULL) %>%
  rename(true_value = value) %>%
  filter(target_end_date >= as.Date("2021-12-01"))

test <- read.csv("https://raw.githubusercontent.com/covid19-forecast-hub-europe/covid19-forecast-hub-europe/main/data-processed/epiforecasts-weeklygrowth/2022-01-23-epiforecasts-weeklygrowth.csv") %>%
  mutate(
    prediction = value,
    target_end_date = as.Date(target_end_date),
    target_variable = gsub("\\d+ wk ahead (inc \\w+)", "\\1", target),
    .keep = "unused"
  )

data <- scoringutils::merge_pred_and_obs(test, truth, "full") %>%
  filter(location == "RO", target_variable == "inc case")

scoringutils::plot_predictions(data, x = "target_end_date") +
    scale_y_continuous(limits = range(data$true_value, na.rm = TRUE), expand = expansion(mult = c(0, 2)), oob = scales::oob_keep)

Now, given that this can be added as an independent ggplot2 layer, does it need to be integrated inside scoringutils::plot_predictions()? We could just document how to do it somewhere. I'm reluctant of adding more complexity for things that could just be extra layers.

[nikosbosse]

Nice! :tada: Doing that without adding code to plot_predictions() is probably a good idea.

[Bisaloo]

Current status: the proposed fix in https://github.com/epiforecasts/scoringutils/pull/107#issuecomment-1031684233 forces the same scale for all facets, which might not be what we want.

[nikosbosse]

@Bisaloo @sbfnk can this be closed?

[sbfnk]

@Bisaloo @sbfnk can this be closed?

Not really - there is still no better solution, and I still wouldn't recommend merging this. I can't think of a better solution than continuing with the occasional rebase.

[seabbs]

This is pretty stuck/dead? I'd suggest closing this as PR but keeping the branch around.

[nikosbosse]

@sbfnk @Bisaloo is this PR needed for anything? Or are you referencing to the branch whenever you need it somewhere?

[seabbs]

So I was suggesting we close the PR but keep the branch. It just isn't great having PRs sitting around with no intention to merge but can keep the suggested solution on a branch with no issue.

[sbfnk]

Fine by me - we're no longer using this but keeping the branch for future reference would still be a good thing I think.