[FEAT] warn when CapeCod.predict infers the grain rather than regrouping silently - #1306
Open
ppcvote wants to merge 1 commit into
Open
[FEAT] warn when CapeCod.predict infers the grain rather than regrouping silently#1306ppcvote wants to merge 1 commit into
ppcvote wants to merge 1 commit into
Conversation
…ing silently predict() re-estimates the apriori at the fitted grain whenever sample_weight carries index levels the fitted apriori_ does not, and returns apriori_ at that grain rather than the caller's. A 775 row triangle goes in and a 6 row apriori_ comes back with nothing said about it. The warning is scoped to self.groupby is None. CapeCod(groupby=...) reaches the same branch, but there the grain was asked for rather than inferred, and warning someone who was explicit is the fastest way to get a warning suppressed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ppcvote
requested review from
genedan,
henrydingliu and
kennethshsu
as code owners
September 6, 2026 06:10
Pyright Type CompletenessView the full Project (full
Other symbols referenced but not exported by
Symbols without documentation:
Patch (exported symbols added or changed by this PR): 0.0% fully typed (0 / 3)
Patch symbol details
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1306 +/- ##
==========================================
+ Coverage 91.69% 91.71% +0.01%
==========================================
Files 96 96
Lines 5456 5466 +10
Branches 702 703 +1
==========================================
+ Hits 5003 5013 +10
Misses 328 328
Partials 125 125
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Summary of Changes
CapeCod.predictre-estimates the apriori at the fitted grain wheneversample_weightcarries index levels the fittedapriori_does not, and returnsapriori_at that grain rather than the caller's. Onclrdthat means a 775 row triangle goes in and a 6 rowapriori_comes back, with nothing said about it. This says it:One thing worth flagging, because it decides the shape of the check.
CapeCod(groupby=...)reaches the same branch: onclrd,CapeCod(groupby="LOB").fit(tri, sample_weight=prem)leavesapriori_at['LOB']whilesample_weightis at['GRNAME', 'LOB'], so the level difference is non-empty and the branch fires. Warning there would fire on someone who already said what they wanted, which is the quickest way to get a warning suppressed wholesale. So the warning is scoped toself.groupby is None: it points out an inferred grain and never a declared one. To be explicit about what that means at the edges,groupbyis a statement of what to keep, so a level it does not name is collapsed by the declaration rather than by inference and stays quiet: withgroupby="LOB"and a prediction triangle at['GRNAME', 'LOB', 'Foo'],apriori_comes back at['LOB']with no warning, which is what was asked for. Going the other way, a level that happens to carry a single value still warns, becauseapriori_still comes back at differentkey_labelsfrom the ones passed in.Related GitHub Issue(s)
The warning discussed in #1274. The
Chainladder.predictshape changes from that same thread are separate and tracked in #1288.Additional Context for Reviewers
self.groupby is Noneguard fails the second. Both halves are covered rather than just the happy path.pytest chainladderpasses: 1142 passed, 7 skipped.ruff check --force-exclude --config lint.per-file-ignores={}passes on both files.ruff format --checkreports exactly what it reported onmainbefore this change, line for line, so the added code introduces nothing of its own.test_capecod_predict1andtest_capecod_predict2, the [BUG] CapeCod predict at different index grain #400 regression tests, now emit the warning, as does the test added in [FIX] CapeCod.predict discards the fitted apriori when the prediction data has exactly one extra index level #1275. That is the behaviour the issue is about and they still pass. Running the file with-W error::UserWarningis a quick way to see exactly which tests take the inferred path.predictdocstring example usesukmotorat a single grain, so the level difference is empty and it neither warns nor changes its documented output.I have not touched the docstrings. Say the word if you would like the behaviour written up there as well.
Checklist
I passed tests locally for both code (
uv run pytest) and documentation changes (uv run --directory docs jb build . --builder=custom --custom-builder=doctest)pytestlocally: 1142 passed, 7 skipped. No documentation changes in this PR, so I did not run the docs doctest build; the one docstring the change could reach is theCapeCod.predictukmotor example, which I ran directly and which still reproduces its documented output.Declarations
I am adhering to the standards in the Governing Doc. I am a human contributor, not a bot, and I opened this because @henrydingliu asked for it on #1274.
AI disclosure, per the AI Usage Policy: I used Claude Code on this. It ran the reproductions, established that
CapeCod(groupby=...)reaches the same branch, wrote the tests and the before and after comparisons. I reviewed the diff and the test file myself, and the suite was run locally on my machine.