<div class="gmail_quote">2009/8/25 Peter Moody <span dir="ltr"><<a href="mailto:peter@hda3.com">peter@hda3.com</a>></span><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">On Mon, Aug 24, 2009 at 3:24 PM, DrKJam<<a href="mailto:drkjam@gmail.com">drkjam@gmail.com</a>> wrote:<br>
</div></blockquote><div><br>[SNIP]<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
As it was left in early June, a pep and design modifications were<br>
requested before ipaddr would be considered for inclusion, but if this<br>
is going to start *another* drawn out ipaddr/netaddr thread, perhaps<br>
the mailman admin(s) could setup a new SIG list for this. I<br>
personally hope that's not required; yours has been the only<br>
dissenting email and I believe I respond to all of your major points<br>
here.</blockquote><div> </div><div>The PEP process is the perfect forum for spending some time scrutinizing and discussing this topic in more detail. I will be raising further points in future when I've had time to fully evaluate both the PEP and the reference implementation of ipaddr.<br>
<br>At this stage, it is premature to assume the reference implementation provided along with the PEP is necessarily complete, only requiring a few bug fixes to get through the approval process.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><div class="h5"><br>
> 1) Firstly, an offering of code.<br>
><br>
> I'd like to bring to your attention an example implementation of an IP<br>
> address library and interface for general discussion to compare and contrast<br>
> with ipaddr 2.0.x :-<br>
><br>
> <a href="http://netaddr.googlecode.com/svn/branches/exp_0.7.x_ip_only" target="_blank">http://netaddr.googlecode.com/svn/branches/exp_0.7.x_ip_only</a><br>
><br>
> It is based on netaddr 0.7.2 which I threw together earlier today.<br>
><br>
> In essence, I've stripped out all of what could be considered non-essential<br>
> code for a purely IP related library. This branch should be suitable for<br>
> *theoretical* consideration of inclusion into some future version of the<br>
> Python standard library (with a little work).<br>
><br>
> It is a pure subset of netaddr release 0.7.2, *minus* the following :-<br>
><br>
> - all IEEE layer-2 code<br>
> - some fairly non-essential IANA IP data files and lookup code<br>
> - IP globbing code (fairly niche)<br>
><br>
> Aside: Just a small mention here that I listened carefully to Clay McClure's<br>
> and others criticisms of the previous incarnation of ipaddr. The 0.7.x<br>
> series of netaddr breaks backward compatibility with previous netaddr<br>
> releases and is an "answer" of sorts to that discussion and issue raised<br>
> within the Python community. I hope you like what I've done with it.<br>
><br>
> For the purposes of this discussion consider this branch the "Firefox to<br>
> netaddr's Mozilla" or maybe just plain old "netaddr-ip-lite" ;-)<br>
><br>
> 2) I refute bold claim in the PEP that :-<br>
><br>
> "Finding a good library for performing those tasks can be somewhat more<br>
> difficult."<br>
><br>
> On the contrary, I wager that netaddr is now a perfectly decent alternative<br>
> implementation to ipaddr, containing quite a few more features with little<br>
> of the slowness for most common operations,<br>
<br>
</div></div>I think you mean refuse, </blockquote><div><br>No, I meant refute.<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">b/c this certainly wasn't the case when I<br>
started writing ipaddr. IPy existed, but it was far too heavyweight<br>
and restrictive for what I needed (no disrespect to the author(s)<br>
intended). I believe I've an email or two from you wherein you<br>
indicate the same.<br>
<div class="im"></div></blockquote><div><br>The comment made on IPy, to which I believe you are referring, was in response to you incorrectly comparing netaddr and IPy's implementation (assuming conditional logic was used within each method to support IP versioning). As already stated netaddr gets around this with a strategy design pattern approach (apologies to readers for using the "Gang of Four" acronym with regard to this).<br>
<br>IPy is heavyweight? How so? It is a mere 1200 lines including comments and deals with IPv4 and IPv6 addressing, much like ipaddr (albeit with fewer features). There are certainly issues you could raise against it (otherwise we wouldn't be here), but being heavyweight is not one of them.<br>
<br>I would actively encourage authors of said library (Victor Stinner is listed as the current maintainer) to get involved in the discussion of this PEP. It is their legacy that this work is picking up from.<br><br>Incidentally, I've noticed a few bug fix releases come through for IPy on PyPI in the last month so that project certainly seems alive and well.<br>
<br>I think the PEP currently doesn't provide appropriate weight to the efforts of others in this area.<br><br>FYI, here is a wiki entry I've been maintaining for a while now to this end :-<br><br><a href="http://code.google.com/p/netaddr/wiki/YetAnotherPythonIPModule">http://code.google.com/p/netaddr/wiki/YetAnotherPythonIPModule</a><br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="im"><br>
> 2/3x faster in a lot of cases,<br>
> not that we're counting. What a difference a year makes!<br>
> I also rate IPy quite highly even if it is getting a little "long in the tooth".<br>
> For a lot of users, IPy could also be considered a nice, stable API!<br>
<br>
</div>yes, netaddr has sped up quite a bit. It's still slower in many cases<br>
as well. But again, who's timing?<br>
<div class="im"></div></blockquote><div><br>I mention speed and timings as the PEP cites this as one of the benefits of considering the ipaddr reference implementation.<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im"><br>
> By the same token I'm happy to note some convergence between the ipaddr and<br>
> netaddr's various interfaces, particularly in light of discussions and<br>
> arguments put forward by Clay McClure and others. A satisfactory compromise<br>
> between the two however still seems a way off.<br>
><br>
><br>
> 3) I also disagree with the PEP's claim that :-<br>
><br>
> "attempts to combine [IPv4 and IPv6] into one object would be like<br>
> trying to force a round peg into a square hole (or vice versa)".<br>
><br>
> netaddr (and for that matter IPy) cope with this perceived problem<br>
> admirably.<br>
><br>
> netaddr employs a simple variant of the GoF Strategy design pattern (with<br>
> added Python sensibility). In the rare cases where ambiguity exists between<br>
> IPv4 and IPv6 addresses a version parameter may be passed to the constructor<br>
> of the IPAddress class to differentiate between them. Providing an IP<br>
> address version to the constructor also provides a small performance<br>
> improvement.<br>
<br>
</div>I'm not sure what point you're trying to make here. I didn't say it<br>
was impossible, I inferred that there are easier ways. having used<br>
code which crams both types into one object, I found it to be cludgey<br>
and complicated so I designed something different.<br>
</blockquote><div><br>Let me clarify. I am +1 on the specific item in the PEP regarding the need for separate and distinct IPAddress and IPNetwork class interfaces that are not conflated into a single interface. Clay McClure made this point very eloquently. I've done a good bit of experimentation on this since it was mentioned so I am fully aware of the pros and cons of each approach. A brief look at netaddr.ip.lite confirms that on this we both agree.<br>
<br>Where I disagree is on the need to have yet another split in the interface to support different IP versions (and a set of Factory functions to pull it all together again). Hey, another design pattern, also known as the "Factory Method" a.k.a. Virtual Constructor (or in this case a Python function).<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
and as a hardly partial observer, I'll add the explicit address<br>
version you can pass to the IPAddress class, but not the IPNetwork<br>
class, is, odd. it actually seems to slow down object creation (~5%)<br>
except in the case of an int arg (your default is about twice as<br>
slow).<br>
<div class="im"></div></blockquote><div><br>Ah, the issue of speed and timings again. Let's concentrate on getting the interface right before we spend too much effort on optimization. I'm quite happy to do a full speed comparison of major features in both libraries but I don't think that would be a worthwhile use of time just now.<br>
<br>Currently I'm ambivalent on whether an IP(vX)Network class constructor should accept a numerical (i.e. integer) value at all *unless* you explicit state somehow that you want the network aspect to be inferred in some specific way. It isn't a case of just choosing /32 or /128 and having this as the only option. IP (v4) classful rules are still pervasive in the real world. A general case IP library available to the whole Python community should certainly take this into account.<br>
<br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="im"><br>
> IPv4 and IPv6 addresses can be used interchangably throughout netaddr<br>
> without causing issue during operations such as sorting, merging (known in<br>
> the PEP as "address collapsing") or address exclusion.<br>
><br>
> Don't try and do this with the current reference implementation of ipaddr :-<br>
><br>
>>>> collapse_address_list([IPv4Address('1.1.1.1'),<br>
>>>> IPv6Address('::1.1.1.1')])<br>
> [IPv4Network('<a href="http://1.1.1.1/32%27" target="_blank">1.1.1.1/32'</a>)]<br>
><br>
> OUCH! Even if this isn't allowed (according to the documentation), it should<br>
> raise an Exception rather than silently passing through.<br>
><br>
> I actually raised this back in May on the ipaddr bug tracker but it hasn't<br>
> received any attention so far :-<br>
><br>
> <a href="http://code.google.com/p/ipaddr-py/issues/detail?id=18" target="_blank">http://code.google.com/p/ipaddr-py/issues/detail?id=18</a><br>
><br>
> Compare this with netaddr's behaviour :-<br>
><br>
>>>> cidr_merge([IPAddress('1.1.1.1'), IPAddress('::1.1.1.1')])<br>
> [IPNetwork('<a href="http://1.1.1.1/32%27" target="_blank">1.1.1.1/32'</a>), IPNetwork('::<a href="http://1.1.1.1/128%27" target="_blank">1.1.1.1/128'</a>)]<br>
><br>
> That's more like it.<br>
<br>
</div>OUCH! indeed. I'm not even sure that this is a nice corner case<br>
feature, summarizing a single list of mixed ip type objects. with an<br>
extra line or two, this can be done in ipaddr, though 'tis true that<br>
we should now raise an exception and don't (it appears to be something<br>
that was introduced recently). If this is a feature for which<br>
developers are clamoring, I'm all over it. Yours is the first email<br>
I've heard mention it.<br>
<div class="im"></div></blockquote><div><br>I may be the only one raising issues but that shouldn't mean they are any less relevant. There is a whole different feel and thrust behind both interfaces each with their own merits.<br>
</div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="im"><br>
> 4) It may just be me but the design of this latest incarnation of ipaddr<br>
> seems somewhat complicated for so few lines of code. Compared with ipaddr,<br>
> netaddr doesn't use or require multiple inheritance nor a seemingly<br>
> convoluted inheritance heirarchy. There isn't a need for an IP() type<br>
> 'multiplexer' function either (although I might be missing an important use<br>
> case here). But, then again, this may just be my personal preference talking<br>
> here. I prefer composition over inheritance in most cases.<br>
<br>
</div>this basically smacks of more petty attackery from the start. so I'll<br>
reply with, "it's just you".<br>
<br>
if you want to debate the merits of GOF strategy vs. multiple<br>
inheritance, fine. the class inheritance in ipaddr is very clean, and<br>
leaves very little code duplication. The classes are very clearly<br>
named and laid out, and in general are much easier to follow than the<br>
strategy method you've chosen for netaddr.<br>
<div class="im"></div></blockquote><div><br>I realise you've done a lot of work on ipaddr and my observations are not intended as a "petty attackery" as you put it. It was merely to question whether the shift in approach from earlier incarnations of ipaddr to this is the correct path to be taking. I don't think that solely relying on "IS A" via multiple inheritance necessarily brings clarity to this code which, as stated in the PEP, is intended to be simple for other to understand and possibly use as a basis for their own extensions. More on this in future posts.<br>
<br>If you missed it I have diagrammed the class hierarchy and internal layout of each library here for consideration :-<br><br><a href="http://code.google.com/p/netaddr/wiki/PEP3144">http://code.google.com/p/netaddr/wiki/PEP3144</a><br>
</div><div>[SNIP] <br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br><div class="im">
> 5) The ipaddr library is also missing options for expanding various<br>
> (exceedingly common) IP abbreviations.<br>
><br>
>>>> from netaddr import IPNetwork<br>
><br>
>>>> IPNetwork('10/8', True)<br>
> IPNetwork('<a href="http://10.0.0.0/8" target="_blank">10.0.0.0/8</a>')<br>
><br>
> netaddr also handles classful IP address logic, still pervasive throughout<br>
> modern IP stacks :-<br>
><br>
>>>> IPNetwork('192.168.0.1', True)<br>
> IPNetwork('<a href="http://192.168.0.1/24" target="_blank">192.168.0.1/24</a>')<br>
><br>
> Note that these options are disabled by default, to keep up the speed of the<br>
> IPNetwork constructor up for more normal cases.<br>
<br>
</div>these seem like corner case features for the sake of having features,<br>
you don't even seem to put much stock in them. FWIW, I've never seen a<br>
request for something similar. I may say '10 slash 8', but I mean,<br>
'<a href="http://10.0.0.0/8" target="_blank">10.0.0.0/8</a>'. I'm missing the utility here, but I'm open to reasoned<br>
arguments.<br>
<div class="im"></div></blockquote><div><br>I don't see why genuine features should be automatically dismissed as "corner cases".<br><br>If you need proof, here an excerpt from RFC 1918 :-<br><br>^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^<br>
3. Private Address Space<br><br><br> The Internet Assigned Numbers Authority (IANA) has reserved the<br> following three blocks of the IP address space for private<br> internets:<br><br> 10.0.0.0 - 10.255.255.255 (10/8 prefix)<br>
172.16.0.0 - 172.31.255.255 (172.16/12 prefix)<br> 192.168.0.0 - 192.168.255.255 (192.168/16 prefix)<br><br>^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ <br><br>I've also had specific requests from users about this feature, one just in the last week (which only required me to point them towards the available switch argument in IPNetwork constructor to enable the required behaviour).<br>
<br>In netaddr 0.7.x I have chosen *not* to make this expansion the default case because it provides a not insignificant construction penalty for those that are not interested in it (as you have already noted and of which I am aware).<br>
<br>I believe strongly that this *is* an important option for a general use IP address library.<br> <br>[SNIP]<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">
<br>
> There is a lot more to consider here than I can cram into this initial<br>
> message, so I'll hand over to you all for some (hopefully) serious debate.<br>
<br>
</div>I'm always open to serious debate, and patches/bug reports (apologies<br>
for missing your earlier issue. I'm not sure if you were aware, but<br>
ipaddr was undergoing a major re-write at the time and I never got<br>
around to following up).</blockquote><div> </div><div>I note your response to this on the ipaddr bug tracker today, thanks. <br><br>[SNIP]<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">> PS - Why does the References section in the PEP contain links to patches<br>
> already applied to the ipaddr 2.0.x reference implementation?<br>
<br>
</div>There's A link to A patch (singular, both times), which has already<br>
been applied. This link exists b/c, at the time I last updated the<br>
PEP, the patch hadn't been applied as it was still being reviewed.</blockquote><div><br>Thanks for the clarification.<br><br>[SNIP]<br></div><br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
in general, that leads<br>
to fewer bugs like the following:<br>
<br>
>>> help(netaddr.IPNetwork.__init__)<br>
Help on method __init__ in module netaddr.ip:<br>
<br>
__init__(self, addr, implicit_prefix=False) unbound netaddr.ip.IPNetwork method<br>
Constructor.<br>
<br>
@param addr: an IPv4 or IPv6 address with optional CIDR prefix,<br>
netmask or hostmask. May be an IP address in representation<br>
(string) format, an integer or another IP object (copy<br>
construction).<br>
<br>
@param implicit_prefix: if True, the constructor uses classful IPv4<br>
rules to select a default prefix when one is not provided.<br>
If False it uses the length of the IP address version.<br>
(default: False).<br>
<br>
>>> netaddr.IPNetwork(1)<br>
<div class="im">Traceback (most recent call last):<br>
File "<stdin>", line 1, in <module><br>
</div> File "./netaddr/ip/__init__.py", line 632, in __init__<br>
prefix, suffix = addr.split('/')<br>
AttributeError: 'int' object has no attribute 'split'<br>
<br>
vs.<br>
<br>
>>> import ipaddr<br>
>>> ipaddr.IPNetwork(1)<br>
IPv4Network('<a href="http://0.0.0.1/32" target="_blank">0.0.0.1/32</a>')<br>
</blockquote><div><br>Thanks for raising this on the netaddr bug tracker. I'll take a look at it. <br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Did you have any other comments on the PEP?<br>
</blockquote><div><br>Yes I do but they will be coming through in stages unfortunately as I get time to look at this further.<br> <br>David Moss<br></div></div>