[Python-Dev] [Python-checkins] cpython: Issue #14814: addition of the ipaddress module (stage 1 - code and tests)

[Terry Reedy]

On 5/22/2012 3:59 PM, Sandro Tosi wrote:
> Thanks Terry for the review! I've attached a patch to issue14814
> addressing your points; but..
>
> On Sun, May 20, 2012 at 7:18 PM, Terry Reedy<tjreedy at udel.edu>  wrote:
>>> +def _get_prefix_length(number1, number2, bits):
>>> +    """Get the number of leading bits that are same for two numbers.
>>> +
>>> +    Args:
>>> +        number1: an integer.
>>> +        number2: another integer.
>>> +        bits: the maximum number of bits to compare.
>>> +
>>> +    Returns:
>>> +        The number of leading bits that are the same for two numbers.
>>> +
>>> +    """
>>> +    for i in range(bits):
>>> +        if number1>>    i == number2>>    i:
>>
>>
>> This non-PEP8 spacing is awful to read. The double space after the tighter
>> binding operator is actively deceptive. Please use
>>
>>         if number1>>  i == number2>>  i:
>
> I don't see this (and all the other) spacing issue you mentioned. Is
> it possible that your mail client had played some "funny" tricks?

Well, *something* between there and here seems to have. I retrieved the 
patch in FF browser and that line looks fine. It also looks fine when I 
cut and pasted it into a test message from a web mail account to my udel 
account, viewed with same mail client. Sorry for the noise. Glad that 
you do not need to 'fix' anything of this sort.

>>> +    Args:
>>> +        first: the first IPv4Address or IPv6Address in the range.
>>> +        last: the last IPv4Address or IPv6Address in the range.
>>> +
>>> +    Returns:
>>> +        An iterator of the summarized IPv(4|6) network objects.
>>
>> Very clear as to types.
>
> I don't think I get exactly what you mean here.

This docstring clearly says what the input type is instead of the more 
vague 'address'. Also, the output is pretty clearly an iterable of 
IPv#Address objects. I meant to contrast this as a good example compared 
to some of the previous docstrings.

-- 
Terry Jan Reedy