Create an empty calibration set instead of erroring - #582
Merged
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a fallback mechanism for calibration splits that creates empty calibration sets instead of throwing errors when splits are not possible. This addresses cases where there are insufficient observations to create both an analysis set and a calibration set, particularly in time series and small dataset scenarios.
Key changes include:
- Wrapping split creation logic in try/catch blocks to handle failures gracefully
- Adding a new
mock_internal_calibration_split()function to create empty calibration sets - Converting error conditions to warnings with informative messages
- Comprehensive test coverage for all split types with mock split scenarios
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| R/inner_split.R | Core implementation of fallback logic across all split methods and new mock split function |
| tests/testthat/test-inner_split.R | Extensive test coverage for mock split creation across all split types |
| tests/testthat/_snaps/inner_split.md | Updated test snapshots showing warnings instead of errors |
| R/misc.R | Added class parameter to make_splits.data.frame method |
| man/make_splits.Rd | Updated documentation for new class parameter |
| inst/calibration-sets.Rmd | Documentation updates reflecting new behavior |
| NEWS.md | Added PR reference to changelog |
| DESCRIPTION | Version bump |
Comments suppressed due to low confidence (1)
tests/testthat/test-inner_split.R:1205
- [nitpick] The test name is too generic. Consider a more descriptive name like 'mock_internal_calibration_split creates proper mock split structure' to clearly indicate what functionality is being tested.
test_that("can create a mock split", {
topepo
requested changes
Jul 24, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #570
This PR adds a mock calibration split which returns the (outer) analysis set as the (inner) analysis set and an empty, i.e., 0-row calibration set. This is used when the calibration split isn't possible, e.g., if there is only one row in the analysis set.
Below is an example with the Chicago data. A
lookback = 0means we only have one period in the analysis set and thus can't split further using the principles we settled on. This is the slightly less obvious case corresponding to "only one row available to split".Created on 2025-07-23 with reprex v2.1.1