Make output of check forecasts be able to be piped into score

[seabbs]

I love the new workflow and the ability to check forecasts. I wonder if it would be possible though to make a change so that the output of check forecasts can be piped into score.

The workflow I am thinking on here is something like

forecast |>
  check_forecast(verbose = TRUE) |>
  score() |> 
  add_coverage() |> 
  summarise_score()

Here we would check the forecast to scoring and only return warnings (true warnings) or errors based on problems when verbose was false. When true would print info about the forecasts as a side effect.

I realise this would be a slightly different design concept and require some changes. If interested happy to make those.

[nikosbosse]

So currently the function provides two things:

1. It returns a list with several items and
2. there is a print function that prints some context and information.

If I understand correctly, your proposal would allow the user to get the printed information as a side effect (if verbose = TRUE). However, in this proposal there would be no way to also get the list items. I think being able to access these list items is useful.

We quickly talked about creating an S3 object that is essentially a data.table which has the other list items as attributes. And then we could have a similar print method as above. To me as a non-enlightenend non-S3-wizard there seems to be a trade-off between how easy you can access the items currently returned by check_forecasts() and whether or not you can pipe them.

Are there other conceivable ways of doing this?

@Bisaloo, what do you think about this?

[seabbs]

If I understand correctly, your proposal would allow the user to get the printed information as a side effect (if verbose = TRUE). However, in this proposal there would be no way to also get the list items. I think being able to access these list items is useful.

It can/could be output as an attribute for the returned forecast. The the print method could access this and use it to return information?

If showing true warnings/messages/errors for issues with forecasts then this would be less of an issue potentially as well?

What parts of the returns needs to be accessed by the user by the way? Anything that needs to be accessed could be made easier for a user by providing a helper to extract attributes?

non-S3-wizard there seems to be a trade-off between how easy you can access the items currently returned by check_forecasts() and whether or not you can pipe

I am not clear this is really the case? I'll have a look at the current output from check_forecasts() to see what exactly it is returning. Which bits do the user need access to vs just from seeing them from print. I think its literally just the case of turning this,

scoringutils/R/check_forecasts.R

Line 170 in a0577e6

class(check) <- c("scoringutils_check", "list")

into

class(check) <- c("scoringutils_check", "data.table") 

where check is a data.table of forecasts with additional attributes from the checking process?

[Bisaloo]

I've been told in the past (and I now agree) it's confusing to have completely different output & behaviour for the same function depending on a single switch.

I believe in this case, it would be clearer to have two separate functions. One with the current behaviour and one implementing what is proposed here. They could be called check_forecasts() vs preprocess_forecasts(), prepare_forecasts(), reformat_forecasts(), etc. Then, the check_forecasts() function would respect what its name says and check forecasts, but nothing more.

[seabbs]

I've been told in the past (and I now agree) it's confusing to have completely different output & behaviour for the same function depending on a single switch.

I don't think that is being suggested here? Just verbose printing in one mode and not in another?

So you would have:

check_forecasts(forecasts)
data.table with forecasts - no warnings or errors everything is fine.

check_forecasts(forecasts, verbose = TRUE)
# This is an x forecast
# It has blah blah blah
# Some issues to be aware of though that are not critical are:
# - Some NAs
# - Some other things
data.table with forecasts - no warnings or errors everything is fine.

check_forecasts(warning_forecasts, verbose = TRUE)
# This is an x forecast
# It has blah blah blah
# Some issues to be aware of though that are not critical are:
# - Some NAs
# - Some other things
warning(something is wrong and I am scared)
data.table with forecasts

check_forecasts(error_forecasts, verbose = TRUE)
# This is an x forecast
# It has blah blah blah
# Some issues to be aware of though that are not critical are:
# - Some NAs
# - Some other things
error(something is wrong and I refuse to go one) 

[nikosbosse]

So currently you have

check <- check_forecasts(example_quantile)

And you could for example check, whether the forecast unit is what it should be in a pipeline by accessing

> check$forecast_unit
[1] "location"        "target_end_date" "target_type"     "location_name"   "forecast_date"  
[6] "model"           "horizon" 

or you could check whether the unique values differ for any specific column by looking at

> check$unique_values
                   model location target_end_date target_type true_value location_name forecast_date quantile prediction horizon
1: EuroCOVIDhub-ensemble        4              12           2         96             4            11       23       3969       3
2: EuroCOVIDhub-baseline        4              12           2         96             4            11       23       3733       3
3:  epiforecasts-EpiNow2        4              12           2         95             4            11       23       3903       3
4:       UMass-MechBayes        4              12           1         48             4            11       23       1058       3

We may decide this is not useful, but it wouldn't be possible with your suggestion, Sam.

[nikosbosse]

One alternative would be to add the data.table with forecasts to the list that check_forecasts() returns and if the object that gets passed to score() is of class scoringutils_check then it could just use that list element with the checked forecasts. And it could also use the warnings / errors stored in the list to just stop or print something.
It wouldn't be more than three or four lines in score() and maybe not win us a beauty contest, but it's pragmatic?

[seabbs]

We may decide this is not useful, but it wouldn't be possible with your suggestion, Sam.

Isn't all of this displayed by the print function? Also it would still be possible just requiring an attributes call first.

i.e

attributes(check)$forecast_unit
[1] "location"        "target_end_date" "target_type"     "location_name"   "forecast_date"  
[6] "model"   

and so on.

One alternative would be to add the data.table with forecasts to the list that check_forecasts() returns and if the object that gets passed to score() is of class scoringutils_check then it could just use that list element with the checked forecasts. And it could also use the warnings / errors stored in the list to just stop or print something.
It wouldn't be more than three or four lines in score() and maybe not win us a beauty contest, but it's pragmatic?

This is very clunky and is a solid 👎🏻 from me. Functions that take varying inputs are a nightmare (I have implemented lots of times in the past and regret everyone). I'd rather we do nothing than make this change.

Seems like this discussion is getting a bit circular. I think we are all now fairly clear on how the pipable version should work but not if we want it or not? Shall I make a MVP PR and we can test it out in comparison with the current implementation?

[nikosbosse]

Shall I make a MVP PR and we can test it out in comparison with the current implementation?
Yes please, that would be very cool! Thank you :)

[seabbs]

Closed by #154

[nikosbosse]

Check_forecasts is now directly called by score(), so no need to pipe it anymore.