Skip to content

Stop Interval.range after yielding the final endpoint - #1052

Open
maxazure wants to merge 1 commit into
python-pendulum:masterfrom
maxazure:fix/interval-range-final-endpoint
Open

maxazure wants to merge 1 commit into
python-pendulum:masterfrom
maxazure:fix/interval-range-final-endpoint

Conversation

@maxazure

Copy link
Copy Markdown

Interval.range() yields its inclusive endpoint and then calculates another date before checking the bounds again. For example, iterating from 9999-12-30 to 9999-12-31 raises 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:

  • Final regression tests against unmodified master: 42 failed, 30 passed, with both pure Python and the locally compiled Rust extension.
  • Fixed full suite: 1932 passed, 3 skipped, for each backend.
  • mypy: passed (228 source files).
  • prek run --all-files: passed, including Rust formatting and clippy.
  • Other CI platforms/Python versions have not been run locally.

AI assistance: prepared and tested with OpenAI Codex.

Pull Request Check List

  • Added tests for changed code.
  • Updated documentation for changed code (not needed; restores the documented behavior).

Copilot AI balanced review requested due to automatic review settings October 13, 2026 02:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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 break after yielding when start == end and amount != 0, so no post-endpoint date is computed.
  • Preserved the existing infinite zero-step (amount == 0) behavior by guarding the break on amount != 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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants