assert_forecast.forecast_nominal() and assert_forecast.forecast_ordinal() have two related bugs:
Bug 1: the incomplete-forecast error names the wrong forecast
When some forecasts are incomplete (not every outcome has a probability assigned), the error message is supposed to point the user at the first incomplete forecast. It instead names the first complete forecast, and reports NA when every forecast is incomplete.
library(scoringutils)
library(data.table)
dat <- data.table(
model = rep(c("modA", "modB"), times = c(3, 2)),
observed = factor("high", levels = c("low", "medium", "high")),
predicted_label = factor(
c("low", "medium", "high", "low", "medium"),
levels = c("low", "medium", "high")
),
predicted = c(0.2, 0.3, 0.5, 0.4, 0.6)
)
suppressWarnings(as_forecast_nominal(dat))
#> Error in `assert_forecast.forecast_nominal()`:
#> ! Found incomplete forecasts
#> i For a nominal forecast, all possible outcomes must be assigned a probability explicitly.
#> i Found first missing probabilities in the forecast identified by model == modA
modA is the complete forecast; modB is the one missing the "high" outcome. If all forecasts are incomplete, the message reads model == NA because the filter selects zero rows.
Cause: R/class-forecast-nominal.R:101 and R/class-forecast-ordinal.R:107 use
first_issue <- complete[(correct), ..forecast_unit][1]
i.e. they filter for the rows where the completeness check passed, while the intent is the rows where it failed. Same bug in both files.
Bug 2: return value is the (visible) forecast instead of invisible(NULL)
The documented contract of assert_forecast() (R/class-forecast.R) is "Returns NULL invisibly", and the binary/point/quantile/sample methods all end with return(invisible(NULL)). The nominal and ordinal methods instead end with return(forecast[]) (R/class-forecast-nominal.R:114, R/class-forecast-ordinal.R:120), so a top-level assert_forecast() call on a nominal/ordinal forecast auto-prints the entire dataset:
withVisible(assert_forecast(as_forecast_nominal(na.omit(example_nominal))))
#> $value is the full forecast object, $visible is TRUE
No caller in the package consumes the return value (as_forecast_*.default calls assert_forecast(data) and returns data separately), and assert_forecast_generic() does not mutate the data.
Intended fix
Per maintainer decision:
- Invert the filter to
complete[!(correct), ..forecast_unit][1] so the error names the first actually incomplete forecast (in both files).
- Replace
return(forecast[]) with return(invisible(NULL)) in both methods, matching the documentation and all other assert_forecast methods. No deprecation cycle; noted in NEWS.md.
Part of the bug audit in #1189.
assert_forecast.forecast_nominal()andassert_forecast.forecast_ordinal()have two related bugs:Bug 1: the incomplete-forecast error names the wrong forecast
When some forecasts are incomplete (not every outcome has a probability assigned), the error message is supposed to point the user at the first incomplete forecast. It instead names the first complete forecast, and reports
NAwhen every forecast is incomplete.modAis the complete forecast;modBis the one missing the"high"outcome. If all forecasts are incomplete, the message readsmodel == NAbecause the filter selects zero rows.Cause: R/class-forecast-nominal.R:101 and R/class-forecast-ordinal.R:107 use
i.e. they filter for the rows where the completeness check passed, while the intent is the rows where it failed. Same bug in both files.
Bug 2: return value is the (visible) forecast instead of
invisible(NULL)The documented contract of
assert_forecast()(R/class-forecast.R) is "Returns NULL invisibly", and the binary/point/quantile/sample methods all end withreturn(invisible(NULL)). The nominal and ordinal methods instead end withreturn(forecast[])(R/class-forecast-nominal.R:114, R/class-forecast-ordinal.R:120), so a top-levelassert_forecast()call on a nominal/ordinal forecast auto-prints the entire dataset:No caller in the package consumes the return value (
as_forecast_*.defaultcallsassert_forecast(data)and returnsdataseparately), andassert_forecast_generic()does not mutate the data.Intended fix
Per maintainer decision:
complete[!(correct), ..forecast_unit][1]so the error names the first actually incomplete forecast (in both files).return(forecast[])withreturn(invisible(NULL))in both methods, matching the documentation and all otherassert_forecastmethods. No deprecation cycle; noted in NEWS.md.Part of the bug audit in #1189.