close
The Wayback Machine - https://web.archive.org/web/20251017020711/https://github.com/python/cpython/pull/4065
Skip to content

Conversation

csabella
Copy link
Contributor

@csabella csabella commented Oct 20, 2017

Patch by Michel Albert.

https://bugs.python.org/issue20825

Lib/ipaddress.py Outdated
return (self.network_address.is_multicast and
self.broadcast_address.is_multicast)

def _containment_check(self, check, other):
Copy link
Member

@pitrou pitrou Oct 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of switching on check, how about having something like:

@classmethod
def _is_subnet_of(cls, a, b):
    # Implementation here

def subnet_of(self, other):
    return self._is_subnet_of(self, other)

def supernet_of(self, other):
    return self._is_subnet_of(other, self)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. I made these changes, except I made is_subnet_of a staticmethod since it didn't use cls.

@pitrou
Copy link
Member

pitrou commented Oct 21, 2017

Thanks @csabella . Please note you'll have to add a NEWS file using the blurb utility.

Lib/ipaddress.py Outdated
raise TypeError("{} and {} are not of the same version"
.format(a, b))
# Dealing with another network.
if hasattr(b, 'network_address') and hasattr(b, 'broadcast_address'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since _is_subnet_of can be called both ways, you should also test the type for a.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@pitrou
Copy link
Member

pitrou commented Oct 22, 2017

Note the CI failures are unrelated. An external NNTP server we use in our test suite seems to have issues...

@pitrou
Copy link
Member

pitrou commented Oct 22, 2017

I took the liberty of making small last-minute changes. Now I'm waiting for CI results again :-)

@pitrou pitrou merged commit 91dc64b into python:master Oct 22, 2017
@csabella
Copy link
Contributor Author

Thank you!

@csabella csabella deleted the bpo20825 branch October 22, 2017 21:55
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.

4 participants