On Mon, Aug 24, 2009 at 3:24 PM, DrKJam<drkjam@gmail.com> wrote:

As it was left in early June, a pep and design modifications were
requested before ipaddr would be considered for inclusion, but if this
is going to start *another* drawn out ipaddr/netaddr thread, perhaps
the mailman admin(s) could setup a new SIG list for this.  I
personally hope that's not required; yours has been the only
dissenting email and I believe I respond to all of your major points
here.

> 1) Firstly, an offering of code.
>
> I'd like to bring to your attention an example implementation of an IP
> address library and interface for general discussion to compare and contrast
> with ipaddr 2.0.x :-
>
>     http://netaddr.googlecode.com/svn/branches/exp_0.7.x_ip_only
>
> It is based on netaddr 0.7.2 which I threw together earlier today.
>
> In essence, I've stripped out all of what could be considered non-essential
> code for a purely IP related library. This branch should be suitable for
> *theoretical* consideration of inclusion into some future version of the
> Python standard library (with a little work).
>
> It is a pure subset of netaddr release 0.7.2, *minus* the following :-
>
> - all IEEE layer-2 code
> - some fairly non-essential IANA IP data files and lookup code
> - IP globbing code (fairly niche)
>
> Aside: Just a small mention here that I listened carefully to Clay McClure's
> and others criticisms of the previous incarnation of ipaddr. The 0.7.x
> series of netaddr breaks backward compatibility with previous netaddr
> releases and is an "answer" of sorts to that discussion and issue raised
> within the Python community. I hope you like what I've done with it.
>
> For the purposes of this discussion consider this branch the "Firefox to
> netaddr's Mozilla" or maybe just plain old "netaddr-ip-lite" ;-)
>
> 2) I refute bold claim in the PEP that :-
>
>     "Finding a good library for performing those tasks can be somewhat more
> difficult."
>
> On the contrary, I wager that netaddr is now a perfectly decent alternative
> implementation to ipaddr, containing quite a few more features with little
> of the slowness for most common operations,

I think you mean refuse,

b/c this certainly wasn't the case when I
started writing ipaddr. IPy existed, but it was far too heavyweight
and restrictive for what I needed (no disrespect to the author(s)
intended). I believe I've an email or two from you wherein you
indicate the same.

> 2/3x faster in a lot of cases,
> not that we're counting. What a difference a year makes!
> I also rate IPy quite highly even if it is getting a little "long in the tooth".
> For a lot of users, IPy could also be considered a nice, stable API!

yes, netaddr has sped up quite a bit. It's still slower in many cases
as well. But again, who's timing?

> By the same token I'm happy to note some convergence between the ipaddr and
> netaddr's various interfaces, particularly in light of discussions and
> arguments put forward by Clay McClure and others. A satisfactory compromise
> between the two however still seems a way off.
>
>
> 3) I also disagree with the PEP's claim that :-
>
>     "attempts to combine [IPv4 and IPv6] into one object would be like
> trying to force a round peg into a square hole (or vice versa)".
>
> netaddr (and for that matter IPy) cope with this perceived problem
> admirably.
>
> netaddr employs a simple variant of the GoF Strategy design pattern (with
> added Python sensibility). In the rare cases where ambiguity exists between
> IPv4 and IPv6 addresses a version parameter may be passed to the constructor
> of the IPAddress class to differentiate between them. Providing an IP
> address version to the constructor also provides a small performance
> improvement.

I'm not sure what point you're trying to make here. I didn't say it
was impossible, I inferred that there are easier ways. having used
code which crams both types into one object, I found it to be cludgey
and complicated so I designed something different.

and as a hardly partial observer, I'll add the explicit address
version you can pass to the IPAddress class, but not the IPNetwork
class, is, odd. it actually seems to slow down object creation (~5%)
except in the case of an int arg (your default is about twice as
slow).

> IPv4 and IPv6 addresses can be used interchangably throughout netaddr
> without causing issue during operations such as sorting, merging (known in
> the PEP as "address collapsing") or address exclusion.
>
> Don't try and do this with the current reference implementation of ipaddr :-
>
>>>> collapse_address_list([IPv4Address('1.1.1.1'),
>>>> IPv6Address('::1.1.1.1')])
> [IPv4Network('1.1.1.1/32')]
>
> OUCH! Even if this isn't allowed (according to the documentation), it should
> raise an Exception rather than silently passing through.
>
> I actually raised this back in May on the ipaddr bug tracker but it hasn't
> received any attention so far :-
>
>     http://code.google.com/p/ipaddr-py/issues/detail?id=18
>
> Compare this with netaddr's behaviour :-
>
>>>> cidr_merge([IPAddress('1.1.1.1'), IPAddress('::1.1.1.1')])
> [IPNetwork('1.1.1.1/32'), IPNetwork('::1.1.1.1/128')]
>
> That's more like it.

OUCH! indeed. I'm not even sure that this is a nice corner case
feature, summarizing a single list of mixed ip type objects. with an
extra line or two, this can be done in ipaddr, though 'tis true that
we should now raise an exception and don't (it appears to be something
that was introduced recently).  If this is a feature for which
developers are clamoring, I'm all over it. Yours is the first email
I've heard mention it.

> 4) It may just be me but the design of this latest incarnation of ipaddr
> seems somewhat complicated for so few lines of code. Compared with ipaddr,
> netaddr doesn't use or require multiple inheritance nor a seemingly
> convoluted inheritance heirarchy. There isn't a need for an IP() type
> 'multiplexer' function either (although I might be missing an important use
> case here). But, then again, this may just be my personal preference talking
> here. I prefer composition over inheritance in most cases.

this basically smacks of more petty attackery from the start. so I'll
reply with, "it's just you".

if you want to debate the merits of GOF strategy vs. multiple
inheritance, fine. the class inheritance in ipaddr is very clean, and
leaves very little code duplication. The classes are very clearly
named and laid out, and in general are much easier to follow than the
strategy method you've chosen for netaddr.

> 5) The ipaddr library is also missing options for expanding various
> (exceedingly common) IP abbreviations.
>
>>>> from netaddr import IPNetwork
>
>>>> IPNetwork('10/8', True)
> IPNetwork('10.0.0.0/8')
>
> netaddr also handles classful IP address logic, still pervasive throughout
> modern IP stacks :-
>
>>>> IPNetwork('192.168.0.1', True)
> IPNetwork('192.168.0.1/24')
>
> Note that these options are disabled by default, to keep up the speed of the
> IPNetwork constructor up for more normal cases.

these seem like corner case features for the sake of having features,
you don't even seem to put much stock in them. FWIW, I've never seen a
request for something similar. I may say '10 slash 8', but I mean,
'10.0.0.0/8'. I'm missing the utility here, but I'm open to reasoned
arguments.

> There is a lot more to consider here than I can cram into this initial
> message, so I'll hand over to you all for some (hopefully) serious debate.

I'm always open to serious debate, and patches/bug reports (apologies
for missing your earlier issue. I'm not sure if you were aware, but
ipaddr was undergoing a major re-write at the time and I never got
around to following up).

> PS - Why does the References section in the PEP contain links to patches
> already applied to the ipaddr 2.0.x reference implementation?

There's A link to A patch (singular, both times), which has already
been applied. This link exists b/c, at the time I last updated the
PEP, the patch hadn't been applied as it was still being reviewed.

in general, that leads
to fewer bugs like the following:

>>> help(netaddr.IPNetwork.__init__)
Help on method __init__ in module netaddr.ip:

__init__(self, addr, implicit_prefix=False) unbound netaddr.ip.IPNetwork method
   Constructor.

   @param addr: an IPv4 or IPv6 address with optional CIDR prefix,
       netmask or hostmask. May be an IP address in representation
       (string) format, an integer or another IP object (copy
       construction).

   @param implicit_prefix: if True, the constructor uses classful IPv4
       rules to select a default prefix when one is not provided.
       If False it uses the length of the IP address version.
       (default: False).

>>> netaddr.IPNetwork(1)

Traceback (most recent call last):
 File "<stdin>", line 1, in <module>

 File "./netaddr/ip/__init__.py", line 632, in __init__
   prefix, suffix = addr.split('/')
AttributeError: 'int' object has no attribute 'split'

vs.

>>> import ipaddr
>>> ipaddr.IPNetwork(1)
IPv4Network('0.0.0.1/32')

Did you have any other comments on the PEP?