absurdfarce commented on code in PR #1273:
URL:
https://github.com/apache/cassandra-python-driver/pull/1273#discussion_r2892771185
##########
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 honestly didn't realize that would work @bschoening, thanks for mentioning
that!
After looking at it for a bit, though, I'm kind of inclined to leave it as a
sequence if/else strings. My only rationale is that it reads much more clearly
to a casual reader. That might be a personal bias on my part because the tuple
comparison syntax you reference was unfamiliar to me (and I'm open to
reconsideration if that's the case) but the current version immediately conveys
the intent behind the code on even a surface reading... which is probably
beneficial.
--
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]
