Fix #1043: Add print method for multivariate forecasts showing joint_across#1074
Fix #1043: Add print method for multivariate forecasts showing joint_across#1074nikosbosse wants to merge 8 commits intomainfrom
Conversation
…t_across Add print.forecast_sample_multivariate() S3 method that displays which columns are jointly forecasted. The method computes joint_across as setdiff(get_forecast_unit(), get_grouping()) and shows it alongside the forecast type and forecast unit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1074 +/- ##
==========================================
+ Coverage 97.98% 98.00% +0.01%
==========================================
Files 37 37
Lines 1984 2001 +17
==========================================
+ Hits 1944 1961 +17
Misses 40 40 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nikosbosse
left a comment
There was a problem hiding this comment.
CLAUDE: Approve (posted as comment due to GitHub self-review restriction). Clean, focused implementation of print.forecast_sample_multivariate() that correctly computes joint_across via setdiff(get_forecast_unit(x), get_grouping(x)) and displays it in the print output. All 5 tests pass and match specs. Three minor notes: (1) the method duplicates ~30 lines from print.forecast() rather than using NextMethod() — deliberate to insert the "Joint across:" line, but creates maintenance coupling; (2) @importFrom cli cli_inform is missing from the roxygen tag (works via package-level NAMESPACE but not best practice); (3) Test 5 name says "still calls parent print.forecast()" but the implementation replicates the parent's logic instead of calling it — the test itself is valid, just the name is misleading.
- Add fixed = TRUE to grepl calls (fixed_regex_linter) - Add tests for forecast_type and forecast_unit error branches to improve code coverage Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
CLAUDE: Here's an example of the new print output for > print(example_multivariate_sample)
Forecast type: multivariate_sample
Forecast unit:
location, location_name, target_end_date, target_type, forecast_date, model,
and horizon
Joint across:
location and location_name
location location_name target_end_date target_type forecast_date
<char> <char> <Date> <char> <Date>
1: DE Germany 2021-01-02 Cases <NA>
2: DE Germany 2021-01-02 Deaths <NA>
3: DE Germany 2021-01-09 Cases <NA>
...
35624: epiforecasts-EpiNow2 2 24.43419 40 78 260The new "Joint across" line shows which columns vary within each multivariate group (i.e., Also pushed a fix for the lint errors ( |
- Rename print.forecast_sample_multivariate to print.forecast_multivariate_sample to match actual class name - Add print.forecast_multivariate_point with same behavior - Extract shared print_multivariate_forecast() helper - Add tests for multivariate point print method - Fix broken forecast_type test to use mock instead of class manipulation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prevent local Claude Code configuration files from being accidentally committed to the repository. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The joint_across display logic in print.forecast() is now unreachable since multivariate forecasts dispatch to their own dedicated print methods (print.forecast_multivariate_sample and print.forecast_multivariate_point). Removing it eliminates uncovered lines and improves project coverage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@seabbs I think adding joint across to the print output would be nice. Things we could consider in the future
|
CLAUDE:
Summary
print.forecast_multivariate_sample()andprint.forecast_multivariate_point()S3 methods that display which columns are jointly forecastedprint_multivariate_forecast()helper that computesjoint_acrossassetdiff(get_forecast_unit(x), get_grouping(x))and shows it between forecast unit and data table outputprint.forecast_sample_multivariate(old class name) toprint.forecast_multivariate_sample(current class name)Root cause
No dedicated print methods existed for multivariate forecast objects — they fell through to the generic
print.forecast(), which only shows forecast type and forecast unit but has no awareness ofjoint_acrossor the.mv_group_idgrouping structure. Additionally, the original PR had the method named after the old class (forecast_sample_multivariate) so it was never dispatched.What the fix does
Creates
print_multivariate_forecast()inR/class-forecast-multivariate-sample.Rthat:print.forecast()behavior)joint_acrosscolumns from the difference between forecast unit and grouping columnsprint(as.data.table(x))Both
print.forecast_multivariate_sample()andprint.forecast_multivariate_point()are thin wrappers that delegate to this helper.Test plan
get_forecast_typeandget_forecast_unit)Closes #1043
🤖 Generated with Claude Code