What's wrong
FiscalYearGroupedSplitter.split skips a fold with continue when no training
row exists strictly before test_fy - gap_years. Every fixture in
tests/test_model_selection.py has enough fiscal years of history that this
never fires, so the skip path is untested — a splitter silently dropping a fold
short of n_splits is exactly the kind of thing that should be pinned with a
regression test.
Where
| File |
Locate with |
philanthropy/model_selection/_temporal_donor_splitter.py |
grep -n "No training data before this test year" philanthropy/model_selection/_temporal_donor_splitter.py |
What to change
No source change — this is test-only.
- Open
tests/test_model_selection.py and look at the existing split()
fixtures (locate with grep -n "def test_" tests/test_model_selection.py).
- Add a test with fiscal years arranged so the earliest requested test year
has no fiscal year strictly before it once gap_years is applied (e.g.
groups starting at the very first fiscal year present, with n_splits
covering it). Assert the splitter yields fewer than n_splits folds instead
of raising or yielding an empty training set.
Tests to add or extend
- File:
tests/test_model_selection.py
- Add:
test_split_skips_fold_with_no_prior_training_data
Done when
python -m pytest tests/test_model_selection.py --cov=philanthropy.model_selection --cov-report=term-missing -q
philanthropy/model_selection/_temporal_donor_splitter.py reads 100% covered
with an empty Missing column.
What's wrong
FiscalYearGroupedSplitter.splitskips a fold withcontinuewhen no trainingrow exists strictly before
test_fy - gap_years. Every fixture intests/test_model_selection.pyhas enough fiscal years of history that thisnever fires, so the skip path is untested — a splitter silently dropping a fold
short of
n_splitsis exactly the kind of thing that should be pinned with aregression test.
Where
philanthropy/model_selection/_temporal_donor_splitter.pygrep -n "No training data before this test year" philanthropy/model_selection/_temporal_donor_splitter.pyWhat to change
No source change — this is test-only.
tests/test_model_selection.pyand look at the existingsplit()fixtures (locate with
grep -n "def test_" tests/test_model_selection.py).has no fiscal year strictly before it once
gap_yearsis applied (e.g.groupsstarting at the very first fiscal year present, withn_splitscovering it). Assert the splitter yields fewer than
n_splitsfolds insteadof raising or yielding an empty training set.
Tests to add or extend
tests/test_model_selection.pytest_split_skips_fold_with_no_prior_training_dataDone when
philanthropy/model_selection/_temporal_donor_splitter.pyreads 100% coveredwith an empty
Missingcolumn.