collapse_address_list is too slow

See ipaddr issue http://code.google.com/p/ipaddr-py/issues/detail?id=17

[Peter Moody]

Matt,

can you upload a patch against the current version?  there have been some
updates recently.

Cheers,
/peter

[Peter Moody]

can you fix up the docstrings here to be more explicit about Args: Returns: etc?

http://codereview.appspot.com/67107/diff/1004/1006
File ipaddr.py (right):

http://codereview.appspot.com/67107/diff/1004/1006#newcode253
Line 253: ip_bits = addresses[0].max_prefixlen
for internal access, I usually end up using the private variable directly just
to avoid as much overhead as possible.

http://codereview.appspot.com/67107/diff/1004/1006#newcode857
Line 857: def max_prefixlen(self):
this could be a property of BaseIP

[Matthew Flanagan]

Patchset 5 is up to 2x faster than Patchset 4:

$ python summarize_test.py
summarize_address_range(IP('0.0.0.0',host=True),IP('255.255.255.254',host=True)):
27.5853071213 s
FASTER
summarize_address_range(IP('0.0.0.0',host=True),IP('255.255.255.254',host=True)):
13.916675806 s
summarize_address_range(IP('0.0.0.0',host=True),IP('255.255.255.255',host=True)):
1.07070207596 s
FASTER
summarize_address_range(IP('0.0.0.0',host=True),IP('255.255.255.255',host=True)):
0.798449993134 s
summarize_address_range(IP('::',host=True),IP('ffff:ffff:ffff:ffff:ffff:ffff:ffff:fffe',host=True)):
288.603591919 s
FASTER
summarize_address_range(IP('::',host=True),IP('ffff:ffff:ffff:ffff:ffff:ffff:ffff:fffe',host=True)):
118.820824862 s
summarize_address_range(IP('::',host=True),IP('ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff',host=True)):
2.90339279175 s
FASTER
summarize_address_range(IP('::',host=True),IP('ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff',host=True)):
1.67668795586 s

[Peter Moody]

http://codereview.appspot.com/67107/diff/2006/2008
File ipaddr.py (right):

http://codereview.appspot.com/67107/diff/2006/2008#newcode252
Line 252: raise IPTypeError("first and last must be IP addresses not networks.")
..." must be IP addresses, not networks"

http://codereview.appspot.com/67107/diff/2006/2008#newcode356
Line 356: elif isinstance(ip, BaseNet):
i think you can skip this elif check and just make it an else:

http://codereview.appspot.com/67107/diff/2006/2007
File ipaddr_test.py (right):

http://codereview.appspot.com/67107/diff/2006/2007#newcode601
Line 601: hash(ipaddr.IP('10.1.1.0', host=True)))
can you verify that your diff is against branches/2.0.x?  I don't see this in:
http://code.google.com/p/ipaddr-py/source/browse/branches/2.0.x/ipaddr_test.py
and Rietveld thinks you're base is svn/trunk

[Matthew Flanagan]

Fixes in Patch Set 7

http://codereview.appspot.com/67107/diff/2006/2008
File ipaddr.py (right):

http://codereview.appspot.com/67107/diff/2006/2008#newcode252
Line 252: raise IPTypeError("first and last must be IP addresses not networks.")
On 2009/08/20 18:21:17, Peter Moody wrote:
> ..." must be IP addresses, not networks"

Done.

http://codereview.appspot.com/67107/diff/2006/2008#newcode356
Line 356: elif isinstance(ip, BaseNet):
On 2009/08/20 18:21:17, Peter Moody wrote:
> i think you can skip this elif check and just make it an else:

Done.

http://codereview.appspot.com/67107/diff/2006/2007
File ipaddr_test.py (right):

http://codereview.appspot.com/67107/diff/2006/2007#newcode601
Line 601: hash(ipaddr.IP('10.1.1.0', host=True)))
On 2009/08/20 18:21:17, Peter Moody wrote:
> can you verify that your diff is against branches/2.0.x?  I don't see this in:
> http://code.google.com/p/ipaddr-py/source/browse/branches/2.0.x/ipaddr_test.py
> and Rietveld thinks you're base is svn/trunk

These are new tests because I added __hash__() to BaseIP so that set() would
work for IP addresses.

[Peter Moody]

On 2009/08/21 01:02:12, Matthew Flanagan wrote:
> 

LGTM.

i'll apply/commit this.