Skip to content

Fix Version.difference with a local variant of the same version - #953

Open
apoorvdarshan wants to merge 2 commits into
python-poetry:mainfrom
apoorvdarshan:fix-10965-local-version-difference
Open

Fix Version.difference with a local variant of the same version#953
apoorvdarshan wants to merge 2 commits into
python-poetry:mainfrom
apoorvdarshan:fix-10965-local-version-difference

Conversation

@apoorvdarshan

@apoorvdarshan apoorvdarshan commented Jul 2, 2026

Copy link
Copy Markdown

Related: python-poetry/poetry#10965

  • Added tests for changed code.
  • Updated documentation for changed code. (internal constraint-arithmetic fix, no user-facing docs)

Problem

A public Version uses PEP 440 weak equality and therefore allows all local variants of the same public version:

public = Version.parse("2.12.1")
local = Version.parse("2.12.1+cpu")
difference = public.difference(local)

Previously difference returned public unchanged, so it still allowed the version that had just been subtracted. Constraint solving could therefore make no progress.

Fix

Represent the public version and all its local variants as a half-open range ending at the next post release (or next development release for dev versions), then subtract the requested local version. This preserves local variants on both sides of the removed value while excluding post/dev successors.

Verification

  • tests/constraints/version/test_version.py: 206 passed.
  • tests/constraints/: 1001 passed.
  • Regression coverage verifies that the removed local variant is rejected, lower and higher local variants remain allowed, and the upper public successor remains rejected.

Scope

This corrects the non-progressing Version.difference() operation identified while investigating python-poetry/poetry#10965. Re-running that issue's full Poetry reproduction now proceeds past the infinite loop but exposes a separate KeyError: Package('triton', '3.7.1') in Poetry's compatibility-result aggregation. This PR no longer claims to fully resolve that Poetry-side failure.

Disclosure

This fix was developed with the assistance of AI tooling; I manually reviewed the constraint semantics, reproduced the original Poetry scenario, and ran the tests above.

Version("2.12.1").difference(Version("2.12.1+cpu")) returned self
unchanged, which still allows 2.12.1+cpu — the subtraction made no
progress, sending poetry's solver into an infinite loop when the same
release was available from multiple sources with different local tags.

Delegate to the equivalent single-point VersionRange, whose difference
arithmetic can represent the split, yielding >=2.12.1,<2.12.1+cpu.

Resolves python-poetry/poetry#10965

@sourcery-ai sourcery-ai Bot 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.

Hey - I've left some high level feedback:

  • In the new difference branch, consider explicitly checking not self.is_local() and other.is_local() (rather than relying solely on self.allows(other)) so the special-case delegation to VersionRange.difference is clearly limited to the non-local vs local-variant scenario and remains robust if allows() semantics evolve.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the new `difference` branch, consider explicitly checking `not self.is_local()` and `other.is_local()` (rather than relying solely on `self.allows(other)`) so the special-case delegation to `VersionRange.difference` is clearly limited to the non-local vs local-variant scenario and remains robust if `allows()` semantics evolve.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@apoorvdarshan

Copy link
Copy Markdown
Author

Re the Sourcery suggestion: at that point in the method other.allows(self) has already been ruled out, so self.allows(other) can only be true when self is non-local and other is a local variant of it (weak equality is the only non-strict path in Version.allows). An explicit not self.is_local() and other.is_local() on its own would also match unrelated versions like 2.12.1 vs 3.0.0+cpu, so the allows() check would still be needed — I've kept the tighter form. Happy to spell out the extra checks alongside if maintainers prefer.

@radoering

Copy link
Copy Markdown
Member

When I try to reproduce the example from python-poetry/poetry#10965 with this fix, I get a KeyError with Package('triton', '3.7.1') (instead of the infinite loop). This change may be correct but it does not seem to really solve the referenced issue.

@apoorvdarshan

Copy link
Copy Markdown
Author

Thanks — I reproduced the full scenario and confirmed the distinction. The original change prevents the non-progressing subtraction, but the lock then fails separately in Poetry's compatibility-result aggregation with KeyError: Package('triton', '3.7.1').

I pushed a follow-up that also fixes the constraint representation: subtracting one local variant now preserves local variants ordered on both sides instead of truncating everything above it. The constraints suite passes (1001 tests). I also changed the PR from “Resolves” to “Related” and documented that it does not by itself solve the remaining Poetry-side aggregation failure.

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