Create custom [.forecast() method

epiforecasts / scoringutils

Utilities for Scoring and Assessing Predictions

https://epiforecasts.io/scoringutils/

Other

48 stars 20 forks source link

Create custom [.forecast() method #884

Closed Bisaloo closed 3 weeks ago

Bisaloo commented 1 month ago

This is a first draft for #816

Todo:

[x] Add custom head() and tail() methods to avoid unnecessary warnings
[x] Add setter methods
[x] Ensure data.table features are conserved in the new method

Open questions / discussion points:

[x] How to reduce duplication in ensuring we manipulate the right class? Maybe a new unclass argument in clean_forecast()?
[x] Double check I'm using data.table correctly and not changing in place when not appropriate

nikosbosse commented 1 month ago

as.data.table() does make a copy by default

nikosbosse commented 1 month ago

I merged new features into main and then thought it would be a good idea to fix the ensuing merge conflicts here. Maybe I should not have done that, apologies for interfering with your code.

This created a test failure due to the fact that the functions for nominal forecasts I implemented modified forecasts objects in a way that triggered your checks.

I made a PR targeting this PR to fix these issues: https://github.com/epiforecasts/scoringutils/pull/892

nikosbosse commented 3 weeks ago

This is great, thanks so much!

f <- as_forecast_quantile(example_quantile)
f <- f |> 
  dplyr::select(-observed)
f

Just noting that we wouldn't get any warnings when people use dplyr. But I assume that is expected/intended, right?

nikosbosse commented 3 weeks ago

It feels like replacing the as.data.table() calls eventually would be nice, but we can also make this an issue and address it later.

seabbs commented 3 weeks ago

oo sorry Nikos I read this first as a approval and not a comment and so hit merge....

I agree as should look at the data.table issue separately.

nikosbosse commented 3 weeks ago

No worries, I think it's fine to merge. @Bisaloo thanks a lot, this was a really cool PR! Do you think there is something clever we can do with dplyr as well? I remember looking into what they use under the hood for mutate at some point. But maybe we also don't want to get too far down the rabbit hole...