Skip to content

perf: Version.__eq__#951

Open
radoering wants to merge 1 commit into
python-poetry:mainfrom
radoering:perf/version-compare
Open

perf: Version.__eq__#951
radoering wants to merge 1 commit into
python-poetry:mainfrom
radoering:perf/version-compare

Conversation

@radoering

Copy link
Copy Markdown
Member

Two changes that make comparisons of versions faster:

  1. inline the most common case in Version.__eq__ and put it at the top

    Admittedly, this is a minor regression in terms of maintainability because it kind of duplicates __eq__ of the parent class but I think it is small and "risk-free enough" so that it is acceptable (the "duplicated" code is only one line and, moreover, something that is unlikely to change in the parent class because it is literally the comparison of the compare keys).

  2. use simple types instead of classes in the _compare_key of PEP440Version

    I think this change is just as good as before in terms of maintainability.

@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:

  • Using self.release._compare_key in PEP440Version._make_compare_key tightens coupling to the internal representation of Release; consider exposing a public accessor (e.g. release.compare_key) or helper to avoid depending on a private attribute.
  • The float sentinels (float('inf') / float('-inf')) and -inf in the local segment appear multiple times; consider centralizing them as named constants (similar to _INF_TAG / _NEG_INF_TAG) to avoid magic values and keep comparison semantics consistent.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Using `self.release._compare_key` in `PEP440Version._make_compare_key` tightens coupling to the internal representation of `Release`; consider exposing a public accessor (e.g. `release.compare_key`) or helper to avoid depending on a private attribute.
- The float sentinels (`float('inf')` / `float('-inf')`) and `-inf` in the local segment appear multiple times; consider centralizing them as named constants (similar to `_INF_TAG` / `_NEG_INF_TAG`) to avoid magic values and keep comparison semantics consistent.

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.

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.

1 participant