test: move example reference values into the scripts themselves - #92
test: move example reference values into the scripts themselves#92endolith wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughExample scripts now expose their computed tables, reference values, and tolerances. ChangesExample regression validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #92 +/- ##
=======================================
Coverage 96.37% 96.37%
=======================================
Files 17 17
Lines 496 496
=======================================
Hits 478 478
Misses 18 18
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:
|
PR Summary
|
6633600 to
dfa2315
Compare
dfa2315 to
7b86c18
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_examples.py (1)
45-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse numpydoc docstrings for the changed functions.
Add
ParametersandReturnssections where applicable. Updatetest_exampleto state why it verifies the embedded-reference contract.As per coding guidelines, “Use numpydoc-format docstrings” and “Every test function must have a docstring explaining the behavior it verifies and why.”
Also applies to: 61-67, 73-78, 89-96, 110-112
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_examples.py` around lines 45 - 46, Update the docstrings for _run and the other changed functions, including test_example, to use numpydoc format with applicable Parameters and Returns sections. Ensure every test function explains both the behavior it verifies and why, with test_example explicitly documenting the embedded-reference contract.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_examples.py`:
- Around line 95-122: Update _assert_close to assert that the computed method
keys exactly match the expected method keys, then retain the per-row validation.
In test_example, validate that table and reference_table have identical figure
keys before iterating nested results, while preserving the existing non-nested
path. Add regression coverage for an empty nested table and for a missing
computed figure.
---
Nitpick comments:
In `@tests/test_examples.py`:
- Around line 45-46: Update the docstrings for _run and the other changed
functions, including test_example, to use numpydoc format with applicable
Parameters and Returns sections. Ensure every test function explains both the
behavior it verifies and why, with test_example explicitly documenting the
embedded-reference contract.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c4d10ba-6189-4fbb-bb1c-0796d888b679
📒 Files selected for processing (12)
examples/README.mdexamples/merrill_1984_fig_2c_2d.pyexamples/merrill_1984_fig_2c_2d_updated.pyexamples/merrill_1984_fig_4a_4b.pyexamples/merrill_1984_fig_4a_4b_updated.pyexamples/merrill_1984_table_1_fig_1.pyexamples/merrill_1984_table_2.pyexamples/merrill_1984_table_3_fig_3.pyexamples/merrill_1984_table_4.pyexamples/weber_1977_effectiveness_table.pyexamples/weber_1977_table_4.pytests/test_examples.py
Each example script now defines its own ``reference_table`` (the values its computed ``table`` is checked against, in the script's column order) and a ``tolerance`` (absolute comparison tolerance), so the script doubles as a test. test_examples.py no longer hardcodes REFERENCE_VALUES/TOLERANCES; it just runs each script (in a subprocess) and verifies ``table`` against the script's own ``reference_table``. Fixes #91. Reference provenance (see issue #88): - merrill_1984_table_1/3 and the Weber scripts reproduce the published tables, so their references are the paper's values (for table_1 and table_3 this is the existing ``merrill_table_1`` dict, renamed ``reference_table``). - merrill_1984_table_2/4 and the four figure scripts do not reproduce the papers (up to ~7-9 pp off), so those references are the docstring "Typical result"/"Results with N elections" values, with a comment noting they are a regression guard until the discrepancy is fixed. The figure scripts were restructured to keep a table per sub-figure (keyed by fig label) instead of overwriting a single ``table`` each loop iteration, so both sub-figures are checked. The examples README documents the convention. Co-authored-by: opencode <opencode@anomalyco.ai>
_assert_close now requires the computed methods to match the reference exactly, catching extra rows that were previously ignored. test_example also checks that a figure script's computed figures match its reference figures, so a missing figure can't pass silently. Add unit tests for both helpers and the empty-table/missing-row/missing-figure cases. Co-authored-by: opencode <opencode@anomalyco.ai>
c3d366b to
28cb15e
Compare
Background
The example scripts are stochastic Monte Carlo scripts that verify the simulator against published results (Merrill 1984, Weber 1977). ests/test_examples.py (added in #87) ran each script and compared its computed able against hardcoded REFERENCE_VALUES/TOLERANCES in the test file.
Problem
The reference values were duplicated in the test file, far from the scripts they describe, even though the values already lived in the scripts (e.g. the merrill_table_1 dicts used for plotting, and the "Typical result" docstring tables). Issue #91 asks that the reference values live in the example scripts themselves.
Visible symptoms
Adding a new example script required editing both the script and the test file. The provenance of each reference (paper value vs. the script's own output) was only documented in test comments, not next to the data.
What this PR changes
eference_table (the values its computed able is checked against, in the script's column order) and olerance (absolute comparison tolerance). Fixes actually the examples should double as tests. #91.
eference_table.
eference_table.
Tests
uff check . --select=E9,F63,F7,F82) passes.
Related work
Summary by CodeRabbit
Documentation
Tests