fix: raise typed exceptions instead of asserts in modifier validation#2714
fix: raise typed exceptions instead of asserts in modifier validation#2714henryiii wants to merge 1 commit into
Conversation
Replace bare `assert` statements in histosys_combined, normsys_combined, and staterror_builder with typed exceptions (InvalidInterpCode / InvalidModel) so validation is never silently bypassed under `python -O`. Add tests covering invalid interpcode values for both modifier types. Assisted-by: ClaudeCode:claude-sonnet-4.6
|
Warning Review limit reached
More reviews will be available in 29 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2714 +/- ##
==========================================
- Coverage 98.28% 96.38% -1.90%
==========================================
Files 65 65
Lines 4305 4313 +8
Branches 465 468 +3
==========================================
- Hits 4231 4157 -74
- Misses 46 122 +76
- Partials 28 34 +6
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:
|
🤖 AI text below 🤖
Part of the code review in #2706.
Summary
assert self.interpcode in [...]inhistosys_combined.__init__andnormsys_combined.__init__withraise InvalidInterpCode(msg)so the check is never silently dropped underpython -Oassert (mask_this_sample == masks[modname]).all()instaterror_builder.finalizewithraise InvalidModel(msg)naming the offending modifier, guarding the real invariant that shared staterror parameters must have identical bin masks across samplestests/test_modifiers.pycovering invalid interpcode values for bothhistosysandnormsysTest plan
pytest tests/test_modifiers.py tests/test_pdf.py tests/test_interpolate.py— 144 passed, 6 skippedprek -a --quiet(ruff + pre-commit) — cleantest_histosys_invalid_interpcodeparametrized over["code1", "code4", "code3", "bad"]all raiseInvalidInterpCodetest_normsys_invalid_interpcodeparametrized over["code0", "code2", "code4p", "bad"]all raiseInvalidInterpCode🤖 Generated with Claude Code