Fix MacOSAppVersion ordering - #80
Merged
Merged
Conversation
__lt__ and __le__ OR'd three independent comparisons together rather than
comparing lexicographically, so a version could sort below another in both
directions at once. That is not hypothetical for the versions in play:
14.4 < 14.5 -> True
14.5 < 14.4 -> True # because "1A107s" < "1A89s" as plain strings
LATEST_VERSION is max(VERSIONS), and VERSIONS is built by iterating
os.listdir(), so with two versions bundled the "latest" version would have
depended on directory order. With one version bundled this was invisible,
which is why it survived.
Three separate defects, all fixed by comparing a single tuple:
- Antisymmetry, as above.
- Digit runs inside version strings compared as text, so "1A107s" sorted
before "1A89s" and "7043.0.9" after "7043.0.93". Now split on digit runs
and compared numerically.
- short_version_comparator scaled each component by 10 ** (len - i), so the
score depended on how many components a version had: "14.4.1" scored 14410
against "14.5" at 1450, making the older release sort higher. Replaced with
a plain tuple comparison, which handles differing lengths correctly.
Adds __eq__ and __hash__ (previously two equal versions compared unequal,
since equality fell back to identity) and derives the rest via total_ordering.
Removes short_version_comparator. It was internal - the only references were
inside this module - and its value was not meaningful across versions with
differing component counts.
Verified the new tests fail on the previous implementation: 6 of 9.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0179M4xvAKPGrgKpy4AeCsM7
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Via my friend Claude: