Fix naming inconsistency: forecast_sample_multivariate class vs as_forecast_multivariate_sample constructor

[seabbs-bot]

Summary

transform_forecasts(append = TRUE) fails on forecast_sample_multivariate objects due to a naming inconsistency between the class name and the constructor function.

The bug

The class is named forecast_sample_multivariate but the constructor is as_forecast_multivariate_sample (different word order). When transform_forecasts() with append = TRUE tries to reconstruct a valid forecast object after binding rows, it dynamically constructs the constructor name from the class:

# R/transform-forecasts.R, lines 161-164
fn_name <- paste0("as_forecast_", forecast_type)
fn <- get(fn_name)
out <- suppressWarnings(suppressMessages(do.call(fn, list(out))))

get_forecast_type() strips the forecast_ prefix from forecast_sample_multivariate, yielding sample_multivariate. So fn_name becomes as_forecast_sample_multivariate — but the actual constructor is as_forecast_multivariate_sample. The get() call fails with "object not found".

Naming comparison

Class	Constructor	transform_forecasts looks for	Match?
forecast_quantile	as_forecast_quantile	as_forecast_quantile	Yes
forecast_sample	as_forecast_sample	as_forecast_sample	Yes
forecast_binary	as_forecast_binary	as_forecast_binary	Yes
forecast_sample_multivariate	as_forecast_multivariate_sample	as_forecast_sample_multivariate	No

Proposed fix

Rename the class from forecast_sample_multivariate to forecast_multivariate_sample so it matches the constructor as_forecast_multivariate_sample. This aligns the naming convention so that get_forecast_type() returns multivariate_sample and transform_forecasts() correctly constructs as_forecast_multivariate_sample.

Why it hasn't been caught

The test suite for transform_forecasts() (tests/testthat/test-transform-forecasts.R) only tests on forecast_quantile and forecast_binary objects — never on forecast_sample_multivariate.

Reprex

library(scoringutils)
# This will fail with: object 'as_forecast_sample_multivariate' not found
transform_forecasts(
  example_multivariate_sample,
  fun = log_shift,
  offset = 1,
  append = TRUE
)

Context

Discovered during planning for hubverse-org/hubEvals#94 (adding sample forecast scoring to hubEvals). hubEvals needs to work around this by blocking transform_append = TRUE for compound sample forecasts.

This was opened by a bot. Please ping @seabbs for any questions.

[seabbs]

@nikosbosse @sbfnk any memory if this was an intentional switch in the naming scheme? I can't imagine it was and I think we all just missed it. Which ordering do we want to go with the converter or the class name?

[seabbs-bot]

Additional context from git history

The constructor was originally named as_forecast_sample_multivariate (matching the class) but was renamed to as_forecast_multivariate_sample in commit f017891 ("address comments from Seb"). The class name forecast_sample_multivariate was not updated to match — this is the root cause of the inconsistency.

Proposed fix direction

Rename the class from forecast_sample_multivariate to forecast_multivariate_sample to match the constructor as_forecast_multivariate_sample. This aligns with the naming direction chosen during review. S3 methods (score.forecast_sample_multivariate, assert_forecast.forecast_sample_multivariate, get_metrics.forecast_sample_multivariate) and is_forecast_sample_multivariate() will need corresponding renames.

This was opened by a bot. Please ping @seabbs for any questions.

[seabbs]

@seabbs-bot is on the case