-
Notifications
You must be signed in to change notification settings - Fork 102
test_n_periods uses return instead of assert #743
Copy link
Copy link
Labels
Effort > Brief 🐇Small tasks expected to take a few hours up to a couple of days.Small tasks expected to take a few hours up to a couple of days.Great First Contribution! 🌱Beginner friendly tickets with narrow scope and huge impact. Perfect to join our community!Beginner friendly tickets with narrow scope and huge impact. Perfect to join our community!Impact > Minor 🔷Small, backward compatible change. Treat like a patch release (e.g., 0.5.8 → 0.5.9).Small, backward compatible change. Treat like a patch release (e.g., 0.5.8 → 0.5.9).
Metadata
Metadata
Assignees
Labels
Effort > Brief 🐇Small tasks expected to take a few hours up to a couple of days.Small tasks expected to take a few hours up to a couple of days.Great First Contribution! 🌱Beginner friendly tickets with narrow scope and huge impact. Perfect to join our community!Beginner friendly tickets with narrow scope and huge impact. Perfect to join our community!Impact > Minor 🔷Small, backward compatible change. Treat like a patch release (e.g., 0.5.8 → 0.5.9).Small, backward compatible change. Treat like a patch release (e.g., 0.5.8 → 0.5.9).
Description
chainladder/development/tests/test_development.py::test_n_periods(line 40) ends withreturn xp.all(...)instead ofassert xp.all(...). Pytest 9 emits aPytestReturnNotNoneWarningfor this:More importantly, the test currently does not actually fail when the equality breaks — it just returns a bool that pytest discards. This silently masks regressions in
Development(n_periods=3, average='volume').Reproduction
Proposed fix
One-line change: replace
returnwithasserton line 43. Outside the scope of #664 / #729 (this is a pytest issue, not a pandas 3.0 issue).Note
This is the only test in the package with this bug — I grepped
^ returnintest_*.pyand the other 5 matches are all legitimate fixture returns. Happy to put up a PR.