From b7c229431a78f857829eb04d01f3061768bf4977 Mon Sep 17 00:00:00 2001 From: nikosbosse Date: Sat, 18 Jul 2026 14:44:30 +0200 Subject: [PATCH 1/4] Consolidate score() input preparation into a single helper Add internal prepare_forecast_for_scoring() that performs the input preparation steps previously duplicated across all eight score() methods: cleaning the forecast, determining the forecast unit, validating the metrics and converting to a plain data.table. Validating the metrics inside the helper also locks down a subtle ordering constraint: the default `metrics = get_metrics(forecast)` promise must be forced while `forecast` still carries its forecast class, i.e. before the rebind to a plain data.table. Closes #941 Co-Authored-By: Claude Fable 5 --- NEWS.md | 1 + R/class-forecast-binary.R | 6 ++-- R/class-forecast-multivariate-point.R | 8 ++---- R/class-forecast-multivariate-sample.R | 8 +++--- R/class-forecast-nominal.R | 8 +++--- R/class-forecast-ordinal.R | 8 +++--- R/class-forecast-point.R | 6 ++-- R/class-forecast-quantile.R | 8 +++--- R/class-forecast-sample.R | 8 +++--- R/score.R | 32 ++++++++++++++++++++++ man/prepare_forecast_for_scoring.Rd | 38 ++++++++++++++++++++++++++ 11 files changed, 100 insertions(+), 31 deletions(-) create mode 100644 man/prepare_forecast_for_scoring.Rd diff --git a/NEWS.md b/NEWS.md index e1ee239eb..71e4d5873 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ # scoringutils (development version) +- Added an internal helper `prepare_forecast_for_scoring()` that consolidates the input preparation steps (cleaning the forecast, determining the forecast unit, validating the metrics and converting to a plain `data.table`) previously duplicated across all `score()` methods (#941). - Added `filter_scores()` and `impute_missing_scores()` for handling missing forecasts before summarisation. `filter_scores()` removes target combinations with insufficient model coverage, while `impute_missing_scores()` fills in missing scores using configurable strategies (worst, mean, NA, or reference model). Both use a strategy function pattern for extensibility. See `vignette("handling-missing-forecasts")` for details (#1122). - Added `plot_discrimination()` to visualise the discrimination ability of binary forecasts by plotting the distribution of predicted probabilities, stratified by the observed outcome. The function requires a `forecast_binary` object (created with `as_forecast_binary()`) (#942). - Fixed `summarise_scores()` producing a data.table with duplicate column names when the input `scores` object had no score columns (e.g. because every metric in `score()` warned and returned nothing). `summarise_scores()` now matches metric columns by exact name rather than regex partial match, and errors with a clear message when there is nothing to summarise (#1179). diff --git a/R/class-forecast-binary.R b/R/class-forecast-binary.R index 5c19c3df0..639e79713 100644 --- a/R/class-forecast-binary.R +++ b/R/class-forecast-binary.R @@ -103,9 +103,9 @@ is_forecast_binary <- function(x) { #' @rdname score #' @export score.forecast_binary <- function(forecast, metrics = get_metrics(forecast), ...) { - forecast <- clean_forecast(forecast, copy = TRUE, na.omit = TRUE) - metrics <- validate_metrics(metrics) - forecast <- as.data.table(forecast) + prep <- prepare_forecast_for_scoring(forecast, metrics) + forecast <- prep$forecast + metrics <- prep$metrics scores <- apply_metrics( forecast, metrics, diff --git a/R/class-forecast-multivariate-point.R b/R/class-forecast-multivariate-point.R index 217249738..581f1e338 100644 --- a/R/class-forecast-multivariate-point.R +++ b/R/class-forecast-multivariate-point.R @@ -107,11 +107,9 @@ is_forecast_multivariate_point <- function(x) { score.forecast_multivariate_point <- function( forecast, metrics = get_metrics(forecast), ... ) { - forecast <- clean_forecast( - forecast, copy = TRUE, na.omit = TRUE - ) - metrics <- validate_metrics(metrics) - forecast <- as.data.table(forecast) + prep <- prepare_forecast_for_scoring(forecast, metrics) + forecast <- prep$forecast + metrics <- prep$metrics observed <- forecast$observed predicted <- matrix(forecast$predicted, ncol = 1) diff --git a/R/class-forecast-multivariate-sample.R b/R/class-forecast-multivariate-sample.R index b44da448a..65907b57c 100644 --- a/R/class-forecast-multivariate-sample.R +++ b/R/class-forecast-multivariate-sample.R @@ -144,10 +144,10 @@ is_forecast_multivariate_sample <- function(x) { #' @rdname score #' @export score.forecast_multivariate_sample <- function(forecast, metrics = get_metrics(forecast), ...) { - forecast <- clean_forecast(forecast, copy = TRUE, na.omit = TRUE) - forecast_unit <- get_forecast_unit(forecast) - metrics <- validate_metrics(metrics) - forecast <- as.data.table(forecast) + prep <- prepare_forecast_for_scoring(forecast, metrics) + forecast <- prep$forecast + metrics <- prep$metrics + forecast_unit <- prep$forecast_unit # transpose the forecasts that belong to the same forecast unit f_transposed <- forecast[, .( diff --git a/R/class-forecast-nominal.R b/R/class-forecast-nominal.R index 187d0bdc0..57e1abdb0 100644 --- a/R/class-forecast-nominal.R +++ b/R/class-forecast-nominal.R @@ -134,10 +134,10 @@ is_forecast_nominal <- function(x) { #' @rdname score #' @export score.forecast_nominal <- function(forecast, metrics = get_metrics(forecast), ...) { - forecast <- clean_forecast(forecast, copy = TRUE, na.omit = TRUE) - forecast_unit <- get_forecast_unit(forecast) - metrics <- validate_metrics(metrics) - forecast <- as.data.table(forecast) + prep <- prepare_forecast_for_scoring(forecast, metrics) + forecast <- prep$forecast + metrics <- prep$metrics + forecast_unit <- prep$forecast_unit # transpose the forecasts that belong to the same forecast unit # make sure the labels and predictions are ordered in the same way diff --git a/R/class-forecast-ordinal.R b/R/class-forecast-ordinal.R index 4af5ab897..fde36039d 100644 --- a/R/class-forecast-ordinal.R +++ b/R/class-forecast-ordinal.R @@ -140,10 +140,10 @@ is_forecast_ordinal <- function(x) { #' @rdname score #' @export score.forecast_ordinal <- function(forecast, metrics = get_metrics(forecast), ...) { - forecast <- clean_forecast(forecast, copy = TRUE, na.omit = TRUE) - forecast_unit <- get_forecast_unit(forecast) - metrics <- validate_metrics(metrics) - forecast <- as.data.table(forecast) + prep <- prepare_forecast_for_scoring(forecast, metrics) + forecast <- prep$forecast + metrics <- prep$metrics + forecast_unit <- prep$forecast_unit # transpose the forecasts that belong to the same forecast unit # make sure the labels and predictions are ordered in the same way diff --git a/R/class-forecast-point.R b/R/class-forecast-point.R index 25e0a97dd..8ca0cde25 100644 --- a/R/class-forecast-point.R +++ b/R/class-forecast-point.R @@ -80,9 +80,9 @@ is_forecast_point <- function(x) { #' @rdname score #' @export score.forecast_point <- function(forecast, metrics = get_metrics(forecast), ...) { - forecast <- clean_forecast(forecast, copy = TRUE, na.omit = TRUE) - metrics <- validate_metrics(metrics) - forecast <- as.data.table(forecast) + prep <- prepare_forecast_for_scoring(forecast, metrics) + forecast <- prep$forecast + metrics <- prep$metrics scores <- apply_metrics( forecast, metrics, diff --git a/R/class-forecast-quantile.R b/R/class-forecast-quantile.R index 7e1ac6246..43279253e 100644 --- a/R/class-forecast-quantile.R +++ b/R/class-forecast-quantile.R @@ -133,10 +133,10 @@ as_forecast_point.forecast_quantile <- function(data, ...) { #' @rdname score #' @export score.forecast_quantile <- function(forecast, metrics = get_metrics(forecast), ...) { - forecast <- clean_forecast(forecast, copy = TRUE, na.omit = TRUE) - forecast_unit <- get_forecast_unit(forecast) - metrics <- validate_metrics(metrics) - forecast <- as.data.table(forecast) + prep <- prepare_forecast_for_scoring(forecast, metrics) + forecast <- prep$forecast + metrics <- prep$metrics + forecast_unit <- prep$forecast_unit # transpose the forecasts that belong to the same forecast unit # make sure the quantiles and predictions are ordered in the same way diff --git a/R/class-forecast-sample.R b/R/class-forecast-sample.R index 27c1f653f..9a0bb9d1c 100644 --- a/R/class-forecast-sample.R +++ b/R/class-forecast-sample.R @@ -129,10 +129,10 @@ as_forecast_quantile.forecast_sample <- function( #' @rdname score #' @export score.forecast_sample <- function(forecast, metrics = get_metrics(forecast), ...) { - forecast <- clean_forecast(forecast, copy = TRUE, na.omit = TRUE) - forecast_unit <- get_forecast_unit(forecast) - metrics <- validate_metrics(metrics) - forecast <- as.data.table(forecast) + prep <- prepare_forecast_for_scoring(forecast, metrics) + forecast <- prep$forecast + metrics <- prep$metrics + forecast_unit <- prep$forecast_unit # transpose the forecasts that belong to the same forecast unit f_transposed <- forecast[, .(predicted = list(predicted), diff --git a/R/score.R b/R/score.R index 037604fab..56e1ff91a 100644 --- a/R/score.R +++ b/R/score.R @@ -132,6 +132,38 @@ score.default <- function(forecast, metrics, ...) { } +#' @title Prepare a forecast object for scoring +#' +#' @description +#' This function performs the input preparation steps shared by all +#' [score()] methods: it validates and cleans the forecast object (removing +#' rows with missing values), determines the forecast unit, validates the +#' metrics, and converts the forecast to a plain `data.table`. +#' +#' The metrics are validated within this function so that the default +#' `metrics = get_metrics(forecast)` in the [score()] methods is evaluated +#' while `forecast` is still a forecast object (method dispatch for +#' [get_metrics()] would fail after the conversion to a plain `data.table`). +#' +#' @inheritParams score +#' @returns A list with three elements: `forecast` (the cleaned forecast as +#' a plain `data.table`), `metrics` (the validated list of metrics) and +#' `forecast_unit` (a character vector with the columns that define the +#' unit of a single forecast). +#' @importFrom data.table as.data.table +#' @keywords internal +prepare_forecast_for_scoring <- function(forecast, metrics) { + forecast <- clean_forecast(forecast, copy = TRUE, na.omit = TRUE) + forecast_unit <- get_forecast_unit(forecast) + metrics <- validate_metrics(metrics) + list( + forecast = as.data.table(forecast), + metrics = metrics, + forecast_unit = forecast_unit + ) +} + + #' @title Apply a list of functions to a data table of forecasts #' @description #' This helper function applies scoring rules (stored as a list of diff --git a/man/prepare_forecast_for_scoring.Rd b/man/prepare_forecast_for_scoring.Rd new file mode 100644 index 000000000..1220a0eda --- /dev/null +++ b/man/prepare_forecast_for_scoring.Rd @@ -0,0 +1,38 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/score.R +\name{prepare_forecast_for_scoring} +\alias{prepare_forecast_for_scoring} +\title{Prepare a forecast object for scoring} +\usage{ +prepare_forecast_for_scoring(forecast, metrics) +} +\arguments{ +\item{forecast}{A forecast object (a validated data.table with predicted and +observed values).} + +\item{metrics}{A named list of scoring functions. Each element should be a +function reference, not a function call. For example, use +\code{list("crps" = crps_sample)} rather than \code{list("crps" = crps_sample())}. +Names will be used as column names in the output. See \code{\link[=get_metrics]{get_metrics()}} for +more information on the default metrics used. See the \emph{Customising metrics} +section below for information on how to pass custom arguments to scoring +functions.} +} +\value{ +A list with three elements: \code{forecast} (the cleaned forecast as +a plain \code{data.table}), \code{metrics} (the validated list of metrics) and +\code{forecast_unit} (a character vector with the columns that define the +unit of a single forecast). +} +\description{ +This function performs the input preparation steps shared by all +\code{\link[=score]{score()}} methods: it validates and cleans the forecast object (removing +rows with missing values), determines the forecast unit, validates the +metrics, and converts the forecast to a plain \code{data.table}. + +The metrics are validated within this function so that the default +\code{metrics = get_metrics(forecast)} in the \code{\link[=score]{score()}} methods is evaluated +while \code{forecast} is still a forecast object (method dispatch for +\code{\link[=get_metrics]{get_metrics()}} would fail after the conversion to a plain \code{data.table}). +} +\keyword{internal} From c875c18cad001de94232d91c945f5242181a9dc6 Mon Sep 17 00:00:00 2001 From: nikosbosse Date: Sat, 18 Jul 2026 14:49:20 +0200 Subject: [PATCH 2/4] Add unit tests for prepare_forecast_for_scoring() Co-Authored-By: Claude Fable 5 --- tests/testthat/test-score.R | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/testthat/test-score.R b/tests/testthat/test-score.R index 44cd7b65f..145c49000 100644 --- a/tests/testthat/test-score.R +++ b/tests/testthat/test-score.R @@ -108,6 +108,42 @@ test_that("function produces output for a nominal format case", { }) +# ============================================================================= +# prepare_forecast_for_scoring() # nolint: commented_code_linter +# ============================================================================= + +test_that("prepare_forecast_for_scoring() prepares a forecast for scoring", { + prep <- prepare_forecast_for_scoring( + example_quantile, get_metrics(example_quantile) + ) + expect_named(prep, c("forecast", "metrics", "forecast_unit")) + + # the forecast is returned as a plain data.table with NA rows removed + expect_s3_class(prep$forecast, c("data.table", "data.frame"), exact = TRUE) + expect_identical(nrow(prep$forecast), nrow(na.omit(example_quantile))) + + # the input is not modified by reference + expect_s3_class(example_quantile, "forecast_quantile") + + expect_identical(prep$metrics, get_metrics(example_quantile)) + expect_identical(prep$forecast_unit, get_forecast_unit(example_quantile)) +}) + +test_that("prepare_forecast_for_scoring() validates its inputs", { + expect_error( + prepare_forecast_for_scoring(data.frame(x = 1), list(mean = mean)), + "forecast object" + ) + expect_warning( + prepare_forecast_for_scoring( + example_binary, + list(brier_score = brier_score, invalid = "not a function") + ), + "not a valid function" + ) +}) + + # ============================================================================= # apply_metrics() # nolint: commented_code_linter # ============================================================================= From 79a6477b754d77c0d9c3ab3cc16808473ded60a1 Mon Sep 17 00:00:00 2001 From: nikosbosse Date: Sat, 18 Jul 2026 14:56:15 +0200 Subject: [PATCH 3/4] Address adversarial review findings - Document precisely why metrics are validated inside prepare_forecast_for_scoring() and that the lazy default is evaluated on the original (pre-cleaning) forecast object. - Describe the metrics argument locally instead of inheriting it from score(), removing a dangling reference to the "Customising metrics" section that only exists on the score() help page. - Reword the NEWS item: determining the forecast unit was previously only done by the methods that need it, not duplicated across all. - Slim the helper tests down to one focused test and make the not-modified-by-reference check a real before/after comparison. Co-Authored-By: Claude Fable 5 --- NEWS.md | 2 +- R/score.R | 14 ++++++++++---- man/prepare_forecast_for_scoring.Rd | 21 ++++++++++----------- tests/testthat/test-score.R | 21 +++------------------ 4 files changed, 24 insertions(+), 34 deletions(-) diff --git a/NEWS.md b/NEWS.md index 71e4d5873..8f0314cd1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,6 @@ # scoringutils (development version) -- Added an internal helper `prepare_forecast_for_scoring()` that consolidates the input preparation steps (cleaning the forecast, determining the forecast unit, validating the metrics and converting to a plain `data.table`) previously duplicated across all `score()` methods (#941). +- Added an internal helper `prepare_forecast_for_scoring()` that consolidates the input preparation steps previously duplicated across the `score()` methods: cleaning the forecast, validating the metrics and converting to a plain `data.table`, plus determining the forecast unit for the methods that need it (#941). - Added `filter_scores()` and `impute_missing_scores()` for handling missing forecasts before summarisation. `filter_scores()` removes target combinations with insufficient model coverage, while `impute_missing_scores()` fills in missing scores using configurable strategies (worst, mean, NA, or reference model). Both use a strategy function pattern for extensibility. See `vignette("handling-missing-forecasts")` for details (#1122). - Added `plot_discrimination()` to visualise the discrimination ability of binary forecasts by plotting the distribution of predicted probabilities, stratified by the observed outcome. The function requires a `forecast_binary` object (created with `as_forecast_binary()`) (#942). - Fixed `summarise_scores()` producing a data.table with duplicate column names when the input `scores` object had no score columns (e.g. because every metric in `score()` warned and returned nothing). `summarise_scores()` now matches metric columns by exact name rather than regex partial match, and errors with a clear message when there is nothing to summarise (#1179). diff --git a/R/score.R b/R/score.R index 56e1ff91a..fffbc8328 100644 --- a/R/score.R +++ b/R/score.R @@ -140,12 +140,18 @@ score.default <- function(forecast, metrics, ...) { #' rows with missing values), determines the forecast unit, validates the #' metrics, and converts the forecast to a plain `data.table`. #' -#' The metrics are validated within this function so that the default -#' `metrics = get_metrics(forecast)` in the [score()] methods is evaluated -#' while `forecast` is still a forecast object (method dispatch for -#' [get_metrics()] would fail after the conversion to a plain `data.table`). +#' The metrics are validated within this function so that the lazy default +#' `metrics = get_metrics(forecast)` of the [score()] methods is forced +#' before those methods rebind `forecast` to a plain `data.table` (there is +#' no `get_metrics.default()`, so forcing the default after that rebind +#' would fail). The default is thereby evaluated on the original forecast +#' object passed to [score()], i.e. before rows with missing values are +#' removed. This makes no difference for the built-in [get_metrics()] +#' methods, which do not inspect the data. #' #' @inheritParams score +#' @param metrics A named list of scoring functions. See [score()] for +#' details. #' @returns A list with three elements: `forecast` (the cleaned forecast as #' a plain `data.table`), `metrics` (the validated list of metrics) and #' `forecast_unit` (a character vector with the columns that define the diff --git a/man/prepare_forecast_for_scoring.Rd b/man/prepare_forecast_for_scoring.Rd index 1220a0eda..1f27f85e0 100644 --- a/man/prepare_forecast_for_scoring.Rd +++ b/man/prepare_forecast_for_scoring.Rd @@ -10,13 +10,8 @@ prepare_forecast_for_scoring(forecast, metrics) \item{forecast}{A forecast object (a validated data.table with predicted and observed values).} -\item{metrics}{A named list of scoring functions. Each element should be a -function reference, not a function call. For example, use -\code{list("crps" = crps_sample)} rather than \code{list("crps" = crps_sample())}. -Names will be used as column names in the output. See \code{\link[=get_metrics]{get_metrics()}} for -more information on the default metrics used. See the \emph{Customising metrics} -section below for information on how to pass custom arguments to scoring -functions.} +\item{metrics}{A named list of scoring functions. See \code{\link[=score]{score()}} for +details.} } \value{ A list with three elements: \code{forecast} (the cleaned forecast as @@ -30,9 +25,13 @@ This function performs the input preparation steps shared by all rows with missing values), determines the forecast unit, validates the metrics, and converts the forecast to a plain \code{data.table}. -The metrics are validated within this function so that the default -\code{metrics = get_metrics(forecast)} in the \code{\link[=score]{score()}} methods is evaluated -while \code{forecast} is still a forecast object (method dispatch for -\code{\link[=get_metrics]{get_metrics()}} would fail after the conversion to a plain \code{data.table}). +The metrics are validated within this function so that the lazy default +\code{metrics = get_metrics(forecast)} of the \code{\link[=score]{score()}} methods is forced +before those methods rebind \code{forecast} to a plain \code{data.table} (there is +no \code{get_metrics.default()}, so forcing the default after that rebind +would fail). The default is thereby evaluated on the original forecast +object passed to \code{\link[=score]{score()}}, i.e. before rows with missing values are +removed. This makes no difference for the built-in \code{\link[=get_metrics]{get_metrics()}} +methods, which do not inspect the data. } \keyword{internal} diff --git a/tests/testthat/test-score.R b/tests/testthat/test-score.R index 145c49000..7e6fc5159 100644 --- a/tests/testthat/test-score.R +++ b/tests/testthat/test-score.R @@ -113,9 +113,8 @@ test_that("function produces output for a nominal format case", { # ============================================================================= test_that("prepare_forecast_for_scoring() prepares a forecast for scoring", { - prep <- prepare_forecast_for_scoring( - example_quantile, get_metrics(example_quantile) - ) + input <- data.table::copy(example_quantile) + prep <- prepare_forecast_for_scoring(input, get_metrics(input)) expect_named(prep, c("forecast", "metrics", "forecast_unit")) # the forecast is returned as a plain data.table with NA rows removed @@ -123,26 +122,12 @@ test_that("prepare_forecast_for_scoring() prepares a forecast for scoring", { expect_identical(nrow(prep$forecast), nrow(na.omit(example_quantile))) # the input is not modified by reference - expect_s3_class(example_quantile, "forecast_quantile") + expect_identical(input, example_quantile) expect_identical(prep$metrics, get_metrics(example_quantile)) expect_identical(prep$forecast_unit, get_forecast_unit(example_quantile)) }) -test_that("prepare_forecast_for_scoring() validates its inputs", { - expect_error( - prepare_forecast_for_scoring(data.frame(x = 1), list(mean = mean)), - "forecast object" - ) - expect_warning( - prepare_forecast_for_scoring( - example_binary, - list(brier_score = brier_score, invalid = "not a function") - ), - "not a valid function" - ) -}) - # ============================================================================= # apply_metrics() # nolint: commented_code_linter From 97db99d389111bcd7e353078d7bccd9ee364dd39 Mon Sep 17 00:00:00 2001 From: nikosbosse Date: Sat, 18 Jul 2026 14:57:24 +0200 Subject: [PATCH 4/4] Add score() test for the ordinal format Scoring an ordinal forecast was not exercised anywhere in the test suite, which codecov surfaced as missing patch coverage on the refactored score.forecast_ordinal() preamble. Co-Authored-By: Claude Fable 5 --- tests/testthat/test-score.R | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/testthat/test-score.R b/tests/testthat/test-score.R index 7e6fc5159..5a50f3f13 100644 --- a/tests/testthat/test-score.R +++ b/tests/testthat/test-score.R @@ -108,6 +108,12 @@ test_that("function produces output for a nominal format case", { }) +# test ordinal case ------------------------------------------------------------ +test_that("function produces output for an ordinal format case", { + expect_no_condition(score(as_forecast_ordinal(na.omit(example_ordinal)))) +}) + + # ============================================================================= # prepare_forecast_for_scoring() # nolint: commented_code_linter # =============================================================================