[radoering]

Follow-up of #953 fixing some more bugs regarding ranges that exclude local versions

Related: python-poetry/poetry#10965

• Added tests for changed code.
• Updated documentation for changed code.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The logic around public versions implicitly representing all local variants is now duplicated between VersionRange.allows_all and Version.difference; consider extracting this into a small helper or method on Version to centralize the semantics and reduce future bugs.
• In VersionRange.allows_all, the special-casing for other.is_local() and the _max.is_local() check could benefit from a brief assertion or guard that documents and enforces the ordering assumptions (e.g., that a local variant always sorts above its base version), making the range behavior easier to reason about.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The logic around public versions implicitly representing all local variants is now duplicated between `VersionRange.allows_all` and `Version.difference`; consider extracting this into a small helper or method on `Version` to centralize the semantics and reduce future bugs.
- In `VersionRange.allows_all`, the special-casing for `other.is_local()` and the `_max.is_local()` check could benefit from a brief assertion or guard that documents and enforces the ordering assumptions (e.g., that a local variant always sorts above its base version), making the range behavior easier to reason about.

## Individual Comments

### Comment 1
<location path="tests/constraints/version/test_version_range.py" line_range="169-178" />
<code_context>
+def test_allows_all_public_version_includes_its_local_variants() -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Add a case where the lower bound is a local variant to guard the implementation’s assumption

`allows_all` explicitly assumes that only the upper bound can be a local variant, since a local lower bound would prevent `other` from ever being allowed. Please add regression tests that cover this, e.g. asserting that `VersionRange(min=local).allows_all(public)` and `VersionRange(min=local, include_min=True).allows_all(public)` both return `False`, to document and verify that a range starting at a local variant can never "allow all" of the corresponding public version.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}