Code review - Issue 67107: collapse_address_list is too slowhttps://codereview.appspot.com/2009-09-11T16:51:14+00:00rietveld
Message from unknown
2009-05-26T00:23:54+00:00Matthew Flanaganurn:md5:2c4b1a531f10204e301b57dbc73bc98c
Message from unknown
2009-05-26T00:34:21+00:00Matthew Flanaganurn:md5:6d7c5604efc1155d4a1f9513a2cd1580
Message from unknown
2009-05-28T08:05:35+00:00Matthew Flanaganurn:md5:e7de332b970be9af8a5eca5c337011ed
Message from pmoody@google.com
2009-06-02T18:27:12+00:00Peter Moodyurn:md5:663c55db244503ef820ba71b7d5f34fc
Matt,
can you upload a patch against the current version? there have been some updates recently.
Cheers,
/peter
Message from pmoody@google.com
2009-06-02T21:05:19+00:00Peter Moodyurn:md5:0e18e4957e5951473ebc7078349eb0da
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
Message from unknown
2009-08-19T11:03:51+00:00Matthew Flanaganurn:md5:6663f6ba2c0d4cfabad5d2de9aabc472
Message from mattimustang@gmail.com
2009-08-19T11:03:58+00:00Matthew Flanaganurn:md5:12c4bbe64bb85ab0c1a313f96d5fe530
Message from unknown
2009-08-19T12:34:53+00:00Matthew Flanaganurn:md5:fbbeda9526138130ce033d9d3256cd45
Message from mattimustang@gmail.com
2009-08-19T12:38:07+00:00Matthew Flanaganurn:md5:c05a0f642e17bb9a9c4549a0fa78fb5e
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
Message from unknown
2009-08-20T00:37:47+00:00Matthew Flanaganurn:md5:4998c1491a7cfc81b2f4935acabcafb1
Message from mattimustang@gmail.com
2009-08-20T00:37:48+00:00Matthew Flanaganurn:md5:93c68071a891fe9fc8ed382f25f17ded
Message from pmoody@google.com
2009-08-20T18:21:17+00:00Peter Moodyurn:md5:47b38c765103de9b7ab1e05bb2a400d2
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
Message from mattimustang@gmail.com
2009-08-21T00:55:56+00:00Matthew Flanaganurn:md5:b763e599dbadd7b8437e8503ea80ccc8
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.
Message from unknown
2009-08-21T01:02:06+00:00Matthew Flanaganurn:md5:3d6b689b4e0c3667d11acd7febc6ac9c
Message from mattimustang@gmail.com
2009-08-21T01:02:12+00:00Matthew Flanaganurn:md5:e54405ee8dacb118985a85aaeb3c6c71
Message from pmoody@google.com
2009-08-21T17:02:25+00:00Peter Moodyurn:md5:834246e645d44c93f55fb2475bd9f849
On 2009/08/21 01:02:12, Matthew Flanagan wrote:
>
LGTM.
i'll apply/commit this.
Message from pmoody@google.com
2009-09-11T16:51:14+00:00Peter Moodyurn:md5:9ba0f8cf2fe7c89c32ce6bb3d227e5d6