-
-
Notifications
You must be signed in to change notification settings - Fork 30k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-29931 fix __lt__ check in ipaddress.ip_interface for both v4 and v6. #879
Conversation
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
@s-sanjay, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ncoghlan, @hynek and @serhiy-storchaka to be potential reviewers. |
Lib/ipaddress.py
Outdated
@@ -1410,11 +1410,17 @@ def __lt__(self, other): | |||
if address_less is NotImplemented: | |||
return NotImplemented | |||
try: | |||
return self.network < other.network | |||
network_less = self.network < other.network |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be written simpler as:
return (self.network < other.network or
self.network == other.network and address_less)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the code.
5326583
to
1569e8e
Compare
the original logic was just comparing the network address but this is wrong because if the network address is equal then we need to compare the ip address for breaking the tie add more ip_interface comparison tests
1569e8e
to
ba99b74
Compare
Please add an entry in |
@serhiy-storchaka updated Misc/NEWs to show this issue |
The sign on the tracker means that Sanjay signed the CLA. That is why I manually changed the label. But he make an error in his GitHub name, sanjay-s instead of s-sanjay. |
@Mariatta @serhiy-storchaka I updated the correct username in issue tracker. thanks for pointing out the typo |
Misc/NEWS
Outdated
@@ -316,6 +319,7 @@ Library | |||
- bpo-25996: Added support of file descriptors in os.scandir() on Unix. | |||
os.fwalk() is sped up by 2 times by using os.scandir(). | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant empty line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
…4 and v6. (pythonGH-879) the original logic was just comparing the network address but this is wrong because if the network address is equal then we need to compare the ip address for breaking the tie add more ip_interface comparison tests. (cherry picked from commit 7bd8d3e)
…4 and v6. (pythonGH-879) the original logic was just comparing the network address but this is wrong because if the network address is equal then we need to compare the ip address for breaking the tie add more ip_interface comparison tests. (cherry picked from commit 7bd8d3e)
the original logic was just comparing the network address
but this is wrong because if the network address is equal then
we need to compare the ip address for breaking the tie
add more ip_interface comparison tests