Repository navigation
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The fix is a minimal, correct boundary-condition guard that preserves documented behavior and is backed by thorough parametrized regression tests.
0 open findings
What changed in this PR
This PR fixes an edge-case bug in Interval.range(). Previously the generator yielded its inclusive endpoint and then unconditionally computed the next date before re-checking the loop bound. At the representable date/time limits (e.g. iterating 9999-12-30 → 9999-12-31, or reverse iteration near year 1), that extra computation overflowed and raised, even though both valid endpoints had already been produced. The fix stops iteration immediately after yielding the endpoint when the step is nonzero.
Changes:
- Added a
breakafter yielding whenstart == endandamount != 0, so no post-endpoint date is computed. - Preserved the existing infinite zero-step (
amount == 0) behavior by guarding the break onamount != 0. - Added parametrized regression tests covering Date/DateTime limits, forward/reverse iteration, absolute intervals, single endpoints, all supported units, larger steps, zero step, and ranges that never land on the endpoint.
| File | Description |
|---|---|
src/pendulum/interval.py |
Adds an early break in range() after yielding the inclusive endpoint to avoid overflow at representable limits. |
tests/interval/test_range.py |
Adds comprehensive parametrized tests for endpoint-stop behavior, zero step, and non-landing steps. |
I reviewed the loop logic against the Interval constructor (how self.start/self.end/invert/absolute are normalized) and confirmed the start == end break is correct for forward, reverse, and absolute cases, and that it only trims the previously overflow-prone trailing computation without changing the inclusive-range output. The zero-step guard correctly preserves the existing infinite-iteration behavior, and the tests accurately assert the expected sequences.
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Interval.range()yields its inclusive endpoint and then calculates another date before checking the bounds again. For example, iterating from9999-12-30to9999-12-31raises after yielding both valid dates; reverse iteration at year 1 has the same problem.Stop after yielding the endpoint when the step is nonzero. This preserves the existing zero-step behavior and does not catch arithmetic errors. Tests cover Date and DateTime limits, forward and reverse iteration, absolute intervals, single endpoints, all supported units, larger steps, and ordinary ranges whose steps do not land on the endpoint.
Scope: this fixes advancement after reaching the endpoint. A step that skips the endpoint can still overflow at the representable limits. The existing documented inclusive range behavior is unchanged, so no documentation update is needed.
Validation on macOS arm64 / CPython 3.13.12:
mypy: passed (228 source files).prek run --all-files: passed, including Rust formatting and clippy.AI assistance: prepared and tested with OpenAI Codex.
Pull Request Check List