absurdfarce commented on code in PR #1273:
URL:
https://github.com/apache/cassandra-python-driver/pull/1273#discussion_r2903629433
##########
cassandra/util.py:
##########
@@ -1757,48 +1746,32 @@ def __repr__(self):
def __str__(self):
return self._version
- @staticmethod
- def _compare_version_part(version, other_version, cmp):
- if not (isinstance(version, int) and
- isinstance(other_version, int)):
- version = str(version)
- other_version = str(other_version)
-
- return cmp(version, other_version)
-
def __eq__(self, other):
if not isinstance(other, Version):
return NotImplemented
return (self.major == other.major and
self.minor == other.minor and
self.patch == other.patch and
- self._compare_version_part(self.build, other.build, lambda s,
o: s == o) and
- self._compare_version_part(self.prerelease, other.prerelease,
lambda s, o: s == o)
+ self.build == other.build and
+ self.prerelease == other.prerelease
)
def __gt__(self, other):
if not isinstance(other, Version):
return NotImplemented
- is_major_ge = self.major >= other.major
- is_minor_ge = self.minor >= other.minor
- is_patch_ge = self.patch >= other.patch
- is_build_gt = self._compare_version_part(self.build, other.build,
lambda s, o: s > o)
- is_build_ge = self._compare_version_part(self.build, other.build,
lambda s, o: s >= o)
-
- # By definition, a prerelease comes BEFORE the actual release, so if a
version
- # doesn't have a prerelease, it's automatically greater than anything
that does
- if self.prerelease and not other.prerelease:
- is_prerelease_gt = False
+ if self.major != other.major:
+ return self.major > other.major
+ elif self.minor != other.minor:
+ return self.minor > other.minor
+ elif self.patch != other.patch:
+ return self.patch > other.patch
+ elif self.build != other.build:
+ return self.build > other.build
+ elif self.prerelease and not other.prerelease:
+ return False
Review Comment:
I got this working @bschoening but it was a little more complicated than it
looked.
Just implementing a tuple comparison brought to light a couple of issues
with the existing impl. We weren't consistently implementing self.prerelease
as a string type which frustrated comparison and equivalence tests. Fixing
that makes the class more consistent generally, so score one for the tests
there.
After that we hit a problem with prereleases. We need to implement custom
sorting here so that identical versions (excluding the prerelease) sort so that
prereleases come first. We don't get that out of a straight tuple compare so I
had to add it manually. This brought back some of the if/else stuff from
before but it's smaller now and (hopefully) still more concise then it was.
Also on the positive side of the ledger; since `__eq__` could also use the
tuple I was able to just add the tuple as a field on the instance and leverage
that across both implementations. I really like where that landed... `__eq__`
is super-simple now.
Anyways, take a look and let me know what you think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
