ipaddress: Interface inheriting from Address

Hi all, I'd like to propose changing ipaddress.IPv[46]Interface to not inherit from IPv[46]Address. (And I hope I've got the right mailing list?) The "ipaddress" module is currently in the standard library on a provisional basis, so "backwards incompatible changes may occur". But obviously there needs to be a very good reason. I think there is. The problem is that IPv[46]Interface can't currently be passed to a function that expects an IPv[46]Address, because it redefines str(), ".exploded", ".compressed", "==" and "<" in an incompatible way. E.g suppose we have:
from ipaddress import IPv4Address, IPv4Interface my_dict = {IPv4Address('1.2.3.4'): 'Hello'}
Obviously lookup with an IPv4Address works:
But with the IPv4Interface subclass, the lookup doesn't work:
And that's because equality has been redefined:
IPv4Address('1.2.3.4') == IPv4Interface('1.2.3.4/24') False
When doing inheritance the usual expectation is that "Functions that use references to base classes must be able to use objects of derived classes without knowing it". This is called the "Liskov Substitution Principle". And IPv4Interface isn't following that principle. More informally, since IPv4Interface inherits from IPv4Address you'd expect that an IPv4Interface "is a" IPv4Address, but it really isn't. It's really a "has a" relationship, which is more commonly done by giving IPv4Interface a property that's an IPv4Address. This problem can't be solved easily by just redefining "==". If we let IPv4Address('1.2.3.4') == IPv4Interface('1.2.3.4/24'), and IPv4Address('1.2.3.4') == IPv4Interface('1.2.3.4/16'), then to keep the normal transitive behaviour of equals we'd have to let IPv4Interface('1.2.3.4/16') == IPv4Interface('1.2.3.4/24'), and that seems wrong. Where people actually know they're comparing IPv4Interface objects they will really want to compare both the IP address and netmask. My proposed solution is just to change IPv4Interface to not inherit from IPv4Address. IPv4Interface already has a "ip" property that gives you the IP address as a proper IPv4Address object. So code written for Python 3.4, using the "ip" property, would be backward-compatible with Python 3.3. And people could obviousy start writing code that way today, to be compatible with both Python 3.3 and Python 3.4. I.e. people should write code like this:
I'm proposing that IPv4Interface would not have a (public) base class, and the only remaining properties and methods on IPv4Interface would be: __init__() ip network compressed exploded with_prefixlen with_netmask with_hostmask __eq__() / __ne__() __hash__() __lt__(), __gt__() etc __str__() And all these properties and methods would do exactly what they do today. I.e. IPv4Interface becomes just a container for the "ip" and "network" fields, plus the parsing code in __init__() and a few formatting and comparison functions. This is a lot simpler to understand than the current design. With this design, IPv4Interface wouldn't be comparable to IPv4Address or IPv4Network objects. If the user wants to do a comparison like that, they can get the .ip or .network properties and compare that. Explicit is better than implicit. If there is interest in this idea, I'll try to put together a patch next week. Thanks to anyone who's read this far. What do you think? Kind regards, Jon

I'm at least mildly in favour of the idea. As you say, containment better reflects the relationship than inheritance. However, it's probably worth checking the ipaddress PEP and related discussions. I seem to recall we considered this approach, but I don't recall *why* we didn't use it. Cheers, Nick.

On Aug 21, 2013, at 15:55, Jon Foster <jon@jon-foster.co.uk> wrote:
But why should you expect these to be equal? The interface is-a address, but isn't _that_ address. Of course an interface isn't equal to _any_ address instance (that isn't also an interface instance), but so what? That's actually _typical_ of class hierarchies, not unusual. Imagine that we had Range with start and stop, and SteppedRange with start, stop, and step. Any SteppedRange with a step != 1 would be unequal to any (non-SteppedRange) Range, but it's still usable as a Range. This is exactly the same situation here. In fact, your example shows that you can store addresses and interfaces together, and use them all as addresses, rather than the opposite--in other words, it fits the LSP perfectly. All that being said, it does feel weird that an interface is an address. While it may satisfy the syntax of an address, it doesn't work behaviorally in most typical uses. For example, socket.connect((str(addr), port)) is the most obvious useful thing to do with an address, and it's not going to work if addr is an interface... So, I think you may have found a real problem, even if your argument for why it's a problem isn't that compelling. Meanwhile, PEP 3144 explains that "ipaddr makes extensive use of inheritance to avoid code duplication...", but that bit doesn't apply here, and I don't see any other justification for the inheritance here in the PEP, docs, or source. So I think we have to dig through the discussions (which may mean going back to the predecessor library's discussions) to see what the intended benefit is.

On 08/21/2013 06:55 PM, Jon Foster wrote:
<stuff deleted> I agree that it's odd that an [x]Interface would inherit from an [x]Address. I think it should be a has-a relationship, as you describe with the "ip" property.
If there is interest in this idea, I'll try to put together a patch next week.
I'd review the patch. Eric.

Hi all, After looking at ipaddress some more, I've got a few patches to suggest. I've pushed them all to a Mercurial repository which you can see here: https://bitbucket.org/jonfoster/python-ipaddress/commits/all?page=3 The "Interface not inheriting from Address" patch is: https://bitbucket.org/jonfoster/python-ipaddress/commits/146d1ffa832fd0b7269... It depends on some refactoring, which I did in a separate commit: https://bitbucket.org/jonfoster/python-ipaddress/commits/af480dbe385f65da3cf... There are a few other ipaddress patches in that repository, too. I'd appreciate any feedback you have on these. Kind regards, Jon P.S. I have just signed a Contributor Agreement. On 27/08/2013 14:18, Eric V. Smith wrote:

I'm at least mildly in favour of the idea. As you say, containment better reflects the relationship than inheritance. However, it's probably worth checking the ipaddress PEP and related discussions. I seem to recall we considered this approach, but I don't recall *why* we didn't use it. Cheers, Nick.

On Aug 21, 2013, at 15:55, Jon Foster <jon@jon-foster.co.uk> wrote:
But why should you expect these to be equal? The interface is-a address, but isn't _that_ address. Of course an interface isn't equal to _any_ address instance (that isn't also an interface instance), but so what? That's actually _typical_ of class hierarchies, not unusual. Imagine that we had Range with start and stop, and SteppedRange with start, stop, and step. Any SteppedRange with a step != 1 would be unequal to any (non-SteppedRange) Range, but it's still usable as a Range. This is exactly the same situation here. In fact, your example shows that you can store addresses and interfaces together, and use them all as addresses, rather than the opposite--in other words, it fits the LSP perfectly. All that being said, it does feel weird that an interface is an address. While it may satisfy the syntax of an address, it doesn't work behaviorally in most typical uses. For example, socket.connect((str(addr), port)) is the most obvious useful thing to do with an address, and it's not going to work if addr is an interface... So, I think you may have found a real problem, even if your argument for why it's a problem isn't that compelling. Meanwhile, PEP 3144 explains that "ipaddr makes extensive use of inheritance to avoid code duplication...", but that bit doesn't apply here, and I don't see any other justification for the inheritance here in the PEP, docs, or source. So I think we have to dig through the discussions (which may mean going back to the predecessor library's discussions) to see what the intended benefit is.

On 08/21/2013 06:55 PM, Jon Foster wrote:
<stuff deleted> I agree that it's odd that an [x]Interface would inherit from an [x]Address. I think it should be a has-a relationship, as you describe with the "ip" property.
If there is interest in this idea, I'll try to put together a patch next week.
I'd review the patch. Eric.

Hi all, After looking at ipaddress some more, I've got a few patches to suggest. I've pushed them all to a Mercurial repository which you can see here: https://bitbucket.org/jonfoster/python-ipaddress/commits/all?page=3 The "Interface not inheriting from Address" patch is: https://bitbucket.org/jonfoster/python-ipaddress/commits/146d1ffa832fd0b7269... It depends on some refactoring, which I did in a separate commit: https://bitbucket.org/jonfoster/python-ipaddress/commits/af480dbe385f65da3cf... There are a few other ipaddress patches in that repository, too. I'd appreciate any feedback you have on these. Kind regards, Jon P.S. I have just signed a Contributor Agreement. On 27/08/2013 14:18, Eric V. Smith wrote:
participants (4)
-
Andrew Barnert
-
Eric V. Smith
-
Jon Foster
-
Nick Coghlan