Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add and document __format__ method for IPv[46]Address #77001

Closed
ewosborne mannequin opened this issue Feb 11, 2018 · 39 comments
Closed

Add and document __format__ method for IPv[46]Address #77001

ewosborne mannequin opened this issue Feb 11, 2018 · 39 comments
Labels
3.9 only security fixes 3.10 only security fixes docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@ewosborne
Copy link
Mannequin

ewosborne mannequin commented Feb 11, 2018

BPO 32820
Nosy @ncoghlan, @ericvsmith, @tiran, @zware, @serhiy-storchaka, @ewosborne, @iritkatriel
PRs
  • bpo-32820: __format__ method for ipaddress #5627
  • bpo-32820: Simplify __format__ implementation for ipaddress. #16378
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2020-10-17.13:07:00.219>
    created_at = <Date 2018-02-11.15:45:54.656>
    labels = ['type-feature', 'library', '3.9', '3.10', 'docs']
    title = 'Add and document __format__ method for IPv[46]Address'
    updated_at = <Date 2020-10-17.15:10:37.808>
    user = 'https://github.com/ewosborne'

    bugs.python.org fields:

    activity = <Date 2020-10-17.15:10:37.808>
    actor = 'ewosborne'
    assignee = 'docs@python'
    closed = True
    closed_date = <Date 2020-10-17.13:07:00.219>
    closer = 'eric.smith'
    components = ['Documentation', 'Library (Lib)']
    creation = <Date 2018-02-11.15:45:54.656>
    creator = 'ewosborne'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32820
    keywords = ['patch']
    message_count = 39.0
    messages = ['311997', '311998', '312005', '312019', '312026', '312043', '312044', '312050', '312052', '312062', '312093', '312120', '312121', '312125', '312156', '312176', '312361', '312396', '312403', '312413', '312414', '312415', '312652', '312654', '312663', '312667', '312698', '313214', '352064', '352065', '353147', '353379', '378783', '378788', '378804', '378809', '378811', '378813', '378821']
    nosy_count = 8.0
    nosy_names = ['ncoghlan', 'eric.smith', 'christian.heimes', 'docs@python', 'zach.ware', 'serhiy.storchaka', 'ewosborne', 'iritkatriel']
    pr_nums = ['5627', '16378']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue32820'
    versions = ['Python 3.9', 'Python 3.10']

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Feb 11, 2018

    This issue adds two things to the ipaddress lib:

    • an __index__ method to allow bin(), oct(), hex() to work
    • a property method to get the fully padded (32-bit or 128-bit) address as a string

    @ewosborne ewosborne mannequin added 3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Feb 11, 2018
    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Feb 11, 2018

    test_not_an_index_issue15559() disallows __index__, so nevermind. Will resubmit without __index__ to get the full binary string, though.

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Feb 11, 2018

    redoing with a bits() property method to return a string, a la:

        def bits(self):
            fstr = '0' + str(IPV4LENGTH) + 'b'
            return '0b' + format(int(self), fstr)

    Works thusly:

    import ipaddress as ip
    a = ip.IPv4Address('0.0.0.42')
    a.bits == '0b00000000000000000000000000101010'

    @ewosborne ewosborne mannequin changed the title Add binary methods to ipaddress Add bits method to ipaddress Feb 11, 2018
    @ericvsmith
    Copy link
    Member

    Without commenting on how useful or desirable this would be, I'll point out the string can be computed as:

    return f'{int(self):#0{IPV4LENGTH+2}b}'

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Feb 12, 2018

    Faster, too.
    My way:
    In [7]: %timeit bits(a)
    1.67 µs ± 7.31 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

    Your way:
    In [11]: %timeit b2(a)
    1.2 µs ± 5.93 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

    I was a little worried about doing it all as a clever one-liner but this
    seems worthwhile. I'll change the code I submitted.
    thanks!

    eric

    On Sun, Feb 11, 2018 at 6:21 PM Eric V. Smith <report@bugs.python.org>
    wrote:

    Eric V. Smith <eric@trueblade.com> added the comment:

    Without commenting on how useful or desirable this would be, I'll point
    out the string can be computed as:

    return f'{int(self):#0{IPV4LENGTH+2}b}'

    ----------
    nosy: +eric.smith


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue32820\>


    @serhiy-storchaka
    Copy link
    Member

    The usefulness of this feature looks questionable to me.

    @tiran
    Copy link
    Member

    tiran commented Feb 12, 2018

    I agree with Serhiy and Eric. It's a needless complication of the module. What's the actual use case of printing a human readable bit representation of an IP address?

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Feb 12, 2018

    It is often useful to have non-decimal representations of IP addresses.
    Hex shows up a lot in sniffer traces, which is why I wanted to provide
    __index__, but that's not going to happen. I use binary a lot when
    teaching subnet masking and address summarization - if you line up bit
    patterns it's much easier to show how things lay out. It's easy enough to
    use bin(int(addr)) but that doesn't zero-pad the string it returns. I find
    myself doing something like

    In [23]: a
    Out[23]: IPv4Address('1.2.3.4')

    In [24]: x = bin(int(a))[2:]

    In [25]: full_x = '0b' + ('0' * (32-len(x)) + x)

    In [26]: full_x
    Out[26]: '0b00000001000000100000001100000100'

    Although, as Eric Smith has pointed out, there's a one liner way to do
    this. But if an IP address can represent itself as an integer (IMO the
    least useful form) it should have at least a binary representation, and
    lack of a seperate __bin__ means this is as close as I could get.

    eric

    On Mon, Feb 12, 2018 at 7:39 AM Christian Heimes <report@bugs.python.org>
    wrote:

    Christian Heimes <lists@cheimes.de> added the comment:

    I agree with Serhiy and Eric. It's a needless complication of the module.
    What's the actual use case of printing a human readable bit representation
    of an IP address?

    ----------
    nosy: +christian.heimes


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue32820\>


    @serhiy-storchaka
    Copy link
    Member

    I wouldn't say that "0b00000000000000010000110110111000100001011010001100000000000000000000000000000000100010100010111000000011011100000111001100110100" is a very human readable. For more readability it is better to group digits by 4 or 8, and why not use hexadecimal then?

    In any case the application of this feature looks pretty narrow to me. And since it can be implemented as a one-line function, I think it shouldn't be added in the stdlib. The ipaddress classes are already complex.

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Feb 12, 2018

    IPv6 is nasty no matter how you do it (cf.
    https://tools.ietf.org/html/rfc1924). And the ipaddr library already has
    hex (packed()).

    Binary's not about direct readabilty, but about ease of comparison. It's
    much easier to show the reader

    '0b00000001000000100000001100000100'
    '0b00000001000000100000001100000011'
    '0b11111111111111111111111111111100'

    and have them figure out whether the mask contains both hosts than to show
    them

    '1.2.3.4'
    '1.2.3.3'
    '255.255.255.252'
    and ask them to convert to binary in their heads. Without the zero padding
    on the left, this is very easy to get wrong.

    But I certainly agree that this is somewhat niche and a convenience
    function, and if the consensus is that this is too narrow for stdlib, so be
    it.

    eric

    On Mon, Feb 12, 2018 at 8:46 AM Serhiy Storchaka <report@bugs.python.org>
    wrote:

    Serhiy Storchaka <storchaka+cpython@gmail.com> added the comment:

    I wouldn't say that
    "0b00000000000000010000110110111000100001011010001100000000000000000000000000000000100010100010111000000011011100000111001100110100"
    is a very human readable. For more readability it is better to group digits
    by 4 or 8, and why not use hexadecimal then?

    In any case the application of this feature looks pretty narrow to me. And
    since it can be implemented as a one-line function, I think it shouldn't be
    added in the stdlib. The ipaddress classes are already complex.

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue32820\>


    @ncoghlan
    Copy link
    Contributor

    I think the aspect that makes this potentially worthy of a helper function is the need to dynamically adjust the field width based on whether you're printing an IPv4 address or an IPv6 one, whether you're printing it in binary or hexadecimal, whether you're printing separator characters, and whether you're printing the base indicator prefix.

    For example, consider the following:

    >>> ip4 = ipaddress.ip_address("1.2.3.4")
    >>> ip4
    IPv4Address('1.2.3.4')
    >>> ip6 = ipaddress.ip_address("::1:2:3:4")
    >>> ip6
    IPv6Address('::1:2:3:4')
    >>> format(int(ip4), "#041_b")
    '0b0000_0001_0000_0010_0000_0011_0000_0100'
    >>> format(int(ip6), "#041_x")
    '0x0000_0000_0000_0000_0001_0002_0003_0004'
    

    The "41" in those examples comes from "prefix_len + (num_bits / bits_per_char) + (num_bits / bits_per_char / chars_per_separator) - 1":

    IPv4 in binary: 2 + (32 / 1) + (32 / 1 / 4) - 1 = 2 + 32 + 8 - 1 = 41
    IPv6 in hex: 2 + (128 / 4) + (128 / 1 / 4) - 1 = 2 + 32 + 8 - 1 = 41

    So I think the potentially interesting method to implement here would be *format*, such that the field width calculation could be made implicit based on the other options selected.

    While backwards compatibility means that IP address formatting would still need to be equivalent to "str(ip)" by default, it would be reasonably straightforward to make "format(ip, 'b')" output the number in binary, "format(ip, 'x')" do it in hex, and "format(ip, 'n')" be equivalent to "b" for IPv4 addresses, and "x" for IPv6 ones.

    Unlike ordinary numbers, the IP addresses would always be zero-padded to a fixed width (32-bits for IPv4, 128 bits for IPv6), but "#" and "_" would be accepted to indicate whether or to add the base indicator prefix and/or group separators.

    Given those enhancements, the display examples above would become:

    >>> format(ip4, "#_n")
    '0b0000_0001_0000_0010_0000_0011_0000_0100'
    >>> format(ip6, "#_n")
    '0x0000_0000_0000_0000_0001_0002_0003_0004'
    

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Feb 13, 2018

    This is an interesting idea. I hacked together something to handle IPv4
    and pushed it to the repository. It works, but I'm afraid it may be kinda
    ugly. Do you have any examples of good, pythonic ways to parse the format
    string or otherwise extend __format__? I didn't see many examples in
    stdlib, and the ones I did got pretty deep into it pretty quick.

    Test output:

    IPv4 address 1.2.3.4
    format: b
    00000001000000100000001100000100

    format: n
    00000001000000100000001100000100

    format: x
    01020304

    format: _b
    0000_0001_0000_0010_0000_0011_0000_0100

    format: _n
    0000_0001_0000_0010_0000_0011_0000_0100

    format: _x
    0102_0304

    format: #b
    0b00000001000000100000001100000100

    format: #n
    0b00000001000000100000001100000100

    format: #x
    0x01020304

    format: #_b
    0b0000_0001_0000_0010_0000_0011_0000_0100

    format: #_n
    0b0000_0001_0000_0010_0000_0011_0000_0100

    format: #_x
    0x0102_0304

    Thanks!

    eric

    On Mon, Feb 12, 2018 at 9:15 PM Nick Coghlan <report@bugs.python.org> wrote:

    Nick Coghlan <ncoghlan@gmail.com> added the comment:

    I think the aspect that makes this potentially worthy of a helper function
    is the need to dynamically adjust the field width based on whether you're
    printing an IPv4 address or an IPv6 one, whether you're printing it in
    binary or hexadecimal, whether you're printing separator characters, and
    whether you're printing the base indicator prefix.

    For example, consider the following:

    >>> ip4 = ipaddress.ip_address("1.2.3.4")
    >>> ip4
    IPv4Address('1.2.3.4')
    >>> ip6 = ipaddress.ip_address("::1:2:3:4")
    >>> ip6
    IPv6Address('::1:2:3:4')
    >>> format(int(ip4), "#041_b")
    '0b0000_0001_0000_0010_0000_0011_0000_0100'
    >>> format(int(ip6), "#041_x")
    '0x0000_0000_0000_0000_0001_0002_0003_0004'
    

    The "41" in those examples comes from "prefix_len + (num_bits /
    bits_per_char) + (num_bits / bits_per_char / chars_per_separator) - 1":

    IPv4 in binary: 2 + (32 / 1) + (32 / 1 / 4) - 1 = 2 + 32 + 8 - 1 = 41
    IPv6 in hex: 2 + (128 / 4) + (128 / 1 / 4) - 1 = 2 + 32 + 8 - 1 = 41

    So I think the potentially interesting method to implement here would be
    *format*, such that the field width calculation could be made implicit
    based on the other options selected.

    While backwards compatibility means that IP address formatting would still
    need to be equivalent to "str(ip)" by default, it would be reasonably
    straightforward to make "format(ip, 'b')" output the number in binary,
    "format(ip, 'x')" do it in hex, and "format(ip, 'n')" be equivalent to "b"
    for IPv4 addresses, and "x" for IPv6 ones.

    Unlike ordinary numbers, the IP addresses would always be zero-padded to a
    fixed width (32-bits for IPv4, 128 bits for IPv6), but "#" and "_" would be
    accepted to indicate whether or to add the base indicator prefix and/or
    group separators.

    Given those enhancements, the display examples above would become:

    >>> format(ip4, "#_n")
    '0b0000_0001_0000_0010_0000_0011_0000_0100'
    >>> format(ip6, "#_n")
    '0x0000_0000_0000_0000_0001_0002_0003_0004'
    

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue32820\>


    @berkerpeksag
    Copy link
    Member

    Eric, please delete the previous message when you reply by email. Keeping the previous message makes your comment harder to read. See https://bugs.python.org/issue32820#msg312050 on browser for example.

    @ericvsmith
    Copy link
    Member

    If you don't recognize the format string, you want to fall back to:

    return super().__format__(fmt)

    Since that will call object.__format__, it will generate an error if fmt is not the empty string, which is what it does currently for IP addresses.

    As far as parsing the format specifier, you're on your own. _pydecimal.py has a version that parses something very similar (or maybe identical) to the mini-language used by float, int, str, etc. But I'm not sure you need to go that far. Since you can define any format spec you'd like, you're not constrained to the mini-language used by the built-in types (see datetime for an example, which just calls strftime). I think the _pydecimal.py approach of using a regex isn't bad, but you might want to delay loading re until __format__ is called.

    Before you go too far down this path, I think this should probably be taken to python-ideas to hash out what the format spec for IP addresses would look like, or even if this is a good idea at all. I'm generally supportive of handling it through __format__().

    @ncoghlan
    Copy link
    Contributor

    Aye, definitely worth a thread on python-ideas. My rationale for suggesting something based on the built-in numeric codes is that it makes it straightforward for *users* to transfer knowledge from that mini-language.

    As far as parsing goes, I was thinking of something along the lines of the following naive approach:

        typechar = fmt[-1:]
        if not typechar or typechar not in ("b", "n", "x"):
            return super().__format__(fmt)
        prefix, group_sep, suffix = fmt[:-1].rpartition("_")
        if prefix and prefix != "#" or suffix:
            return super().__format__(fmt)
        field_width = self._calculate_field_width(typechar)
        return format(int(self), f"{prefix}0{field_width}{group_sep}{type_char}")

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Feb 14, 2018

    Cool, I will kick it over to python-ideas. I checked in some code to
    handle the format string and it's a lot like what you're suggesting, so
    I'll leave that in there and see what happens.
    Thanks!

    eric

    On Tue, Feb 13, 2018 at 11:56 PM Nick Coghlan <report@bugs.python.org>
    wrote:

    Nick Coghlan <ncoghlan@gmail.com> added the comment:

    Aye, definitely worth a thread on python-ideas. My rationale for
    suggesting something based on the built-in numeric codes is that it makes
    it straightforward for *users* to transfer knowledge from that
    mini-language.

    As far as parsing goes, I was thinking of something along the lines of the
    following naive approach:

    typechar = fmt[-1:]
    if not typechar or typechar not in ("b", "n", "x"):
        return super().\_\_format__(fmt)
    prefix, group_sep, suffix = fmt[:-1].rpartition("\_")
    if prefix and prefix != "#" or suffix:
        return super().\_\_format__(fmt)
    field_width = self.\_calculate_field_width(typechar)
    return format(int(self),
    

    f"{prefix}0{field_width}{group_sep}{type_char}")

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue32820\>


    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Feb 19, 2018

    I brought it up on python-ideas. Since I've not been through this process
    before - what happens now? Do I wait for code review on github, or is
    there more I need to do?

    eric

    On Tue, Feb 13, 2018 at 11:56 PM Nick Coghlan <report@bugs.python.org>
    wrote:

    Nick Coghlan <ncoghlan@gmail.com> added the comment:

    Aye, definitely worth a thread on python-ideas. My rationale for
    suggesting something based on the built-in numeric codes is that it makes
    it straightforward for *users* to transfer knowledge from that
    mini-language.

    As far as parsing goes, I was thinking of something along the lines of the
    following naive approach:

    typechar = fmt[-1:]
    if not typechar or typechar not in ("b", "n", "x"):
        return super().\_\_format__(fmt)
    prefix, group_sep, suffix = fmt[:-1].rpartition("\_")
    if prefix and prefix != "#" or suffix:
        return super().\_\_format__(fmt)
    field_width = self.\_calculate_field_width(typechar)
    return format(int(self),
    

    f"{prefix}0{field_width}{group_sep}{type_char}")

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue32820\>


    @ncoghlan
    Copy link
    Contributor

    The python-ideas discussion didn't turn up any major concerns we hadn't already considered, so you're in "wait for PR review" mode now. If you wanted to do a self-review in the meantime, then https://devguide.python.org/committing/#accepting-pull-requests covers the kinds of additional things a reviewer will be looking for.

    @serhiy-storchaka
    Copy link
    Member

    I the bits() method is a dead end. The conclusion was to add __format__().

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Feb 20, 2018

    Yeah, bits() is dead. The thread has the same title, but I changed the PR
    a while ago. The diffs in the PR are for format().

    I'll go over the code and clean it up. The docstring in particular is
    probably lousy.
    Thanks!

    eric

    On Tue, Feb 20, 2018 at 3:35 AM Serhiy Storchaka <report@bugs.python.org>
    wrote:

    Serhiy Storchaka <storchaka+cpython@gmail.com> added the comment:

    I the bits() method is a dead end. The conclusion was to add __format__().

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue32820\>


    @serhiy-storchaka
    Copy link
    Member

    And Eric, please avoid including the quote of previous message in your messages. This makes hard to read your messages and the whole discussion.

    @ncoghlan
    Copy link
    Contributor

    I've updated the issue title to reflect the revised proposal (i.e. using __format__ rather than a new IP address specific method).

    @ncoghlan ncoghlan changed the title Add bits method to ipaddress Add __format__ method to ipaddress Feb 20, 2018
    @ericvsmith
    Copy link
    Member

    I'd like 's' to be supported, and just be passed to format(str(self), fmt) (or some alternate spelling for the same thing).

    My use case is to format tables including addresses:

    print(f'{addr:20s} {hostname}')

    @serhiy-storchaka
    Copy link
    Member

    You always can use f'{addr!s:20}'.

    @ericvsmith
    Copy link
    Member

    True enough. You'd think that I (of all people!) would know that. Nevermind.

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Feb 23, 2018

    I dunno, I think this might be useful. A binary representation is itself a
    string, and takes formatting as such (ditto for hex, as hex(int()) returns
    a string:

    In [20]: a
    Out[20]: IPv4Address('1.2.3.4')

    In [92]: f'{a}'
    Out[92]: '1.2.3.4'

    In [21]: int(a)
    Out[21]: 16909060

    In [22]: f'{bin(int(a)):42s}'
    Out[22]: '0b1000000100000001100000100 '

    In [24]: f'{a:b}'
    Out[24]: '00000001000000100000001100000100'

    In [25]: f'{a:b42s}'
    Out[25]: '1.2.3.4'

    That last one should return '1000000100000001100000100 '. I was
    worried about going down a really deep rabbit hole trying to support a lot
    of string format stuff with no use case, but there's not much more which
    could be done which makes any sense. 's' seems reasonable.

    My current code supports [b, x, n] integer presentation types. I need to
    add [X], that's just an oversight. Supporting [b, x, X, n] means that an
    IP address is considered an integer, and should get the subset of integer
    presentations which make sense. Not the full set - neither octal nor
    character are good fits. But support for some sort of alignment padding
    seems reasonable. Consider:

    In [61]: f'{42:30}'
    Out[61]: ' 42'

    In [62]: f'{int(a):30}'
    Out[62]: ' 16909060'

    In [63]: f'{a:30}'
    Out[63]: '1.2.3.4'

    In [66]: f'{a:42b}'
    Out[66]: '00000001000000100000001100000100'

    Those last two seem odd. I think f'{a:30}' should return the same thing as
    this:

    In [69]: f'{str(a):30}'
    Out[69]: '1.2.3.4 '

    and f'{a:42b'} should return the same thing as this:

    In [77]: f'{bin(int(a)):42}'
    Out[77]: '0b1000000100000001100000100 '

    This also means supporting [>,<,^] alignment. And, of course, ignoring any
    length spec too short, as is done with regular integer formatting:

    In [86]: b
    Out[86]: 16909060

    In [87]: f'{b:6}'
    Out[87]: '16909060'

    Thoughts?

    eric

    @ncoghlan
    Copy link
    Contributor

    I think supporting "s" makes sense, as we're going to need to treat the empty format string as implying "s" for backwards compatibility reasons:

        >>> f"{v4}"
        '1.2.3.4'

    Right now, attempting to format that field without !s will give you a TypeError, but as Eric O notes, once IP addresses support format, we're going to have to choose between:

    • raise an exception if formatting directions are given without a numeric typecode
    • ignore the formatting in that case (which is what the current PR does)
    • handle it the same way "!s" would (and also allow "s" as an explicit type code)

    The last option sounds the most desirable to me, as that way we can just say "'s' is the default IP addressing formatting typecode" (similar to the way 'd' is the default for integers, and 'g' is the default for floating point numbers).

    I *don't* think we should allow combining the numeric typecode with the string type code, though. The numeric types don't allow that, and f-strings already support nesting if you want to combine string formatting with numeric formatting:

        >>> f"{f'{42**100:g}':>20s}"
        '        2.11314e+162'

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Mar 4, 2018

    I have pushed out new diffs.

    • moved __format__ to _BaseAddress, where it should have been in the first
      place
    • redid format parser as regexp, as it was getting awfully complicated
    • added support for 'X'
    • added support for 's' by falling through to regular format()
    • added IPv6 to the test suite

    This was a decent-sized change, but all the tests pass so it looks OK to me.
    As per request, I defer importing re and compiling the necessary regexp
    until it's absolutely necessary. This is significantly slower than
    importing re and compiling the regexp when ipaddress is imported.

    localized compile and import
    In [4]: %timeit f'{a:#_b}'
    7.05 µs ± 34.2 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

    toplevel compile and import
    In [2]: %timeit f'{a:#_b}'
    5.34 µs ± 17 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

    Is this worth trying to get clever about? It doesn't matter in my use case,
    and I could make a reasonable argument either way. I'm tempted to leave it
    localized, as I can't imagine a case where formatting eleventy billion IP
    addresses as padded binary is all that time-sensitive. On the other hand,
    I'm also not sure how Pythonic it is to stick an import statement 20 lines
    deep in a dunder method, so I'm open to suggestions.

    eric

    @zware
    Copy link
    Member

    zware commented Sep 12, 2019

    New changeset f9c95a4 by Zachary Ware (ewosborne) in branch 'master':
    bpo-32820: __format__ method for ipaddress (bpo-5627)
    f9c95a4

    @zware
    Copy link
    Member

    zware commented Sep 12, 2019

    The enhancement patch is merged, but it occurs to me after the fact that this could use some documentation, and possibly a mention in whatsnew. I'll leave this open as a documentation issue.

    @zware zware added 3.9 only security fixes docs Documentation in the Doc dir and removed 3.8 only security fixes labels Sep 12, 2019
    @zware zware changed the title Add __format__ method to ipaddress Add and document __format__ method for IPv[46]Address Sep 12, 2019
    @serhiy-storchaka
    Copy link
    Member

    PR 16378 simplifies the implementation. It also caches the compiled RE.

    Needed the documentation for the new feature.

    @serhiy-storchaka
    Copy link
    Member

    New changeset 5d6f5b6 by Serhiy Storchaka in branch 'master':
    bpo-32820: Simplify __format__ implementation for ipaddress. (GH-16378)
    5d6f5b6

    @iritkatriel
    Copy link
    Member

    Do these PRs need to be backported? They're in 3.9 but not 3.8.

    @ericvsmith
    Copy link
    Member

    Re: backporting

    A quick test shows this feature is not in 3.8. We can't add new features to 3.8, so I'd say "no, it doesn't need to be backported".

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Oct 17, 2020

    Yes, I agree. Regardless of backport policy, this is a handy little
    convenience featurette, not sliced bread v2, so it's not worth backporting
    even if it was permissible.

    eric

    On Fri, Oct 16, 2020 at 7:13 PM Eric V. Smith <report@bugs.python.org>
    wrote:

    Eric V. Smith <eric@trueblade.com> added the comment:

    Re: backporting

    A quick test shows this feature is not in 3.8. We can't add new features
    to 3.8, so I'd say "no, it doesn't need to be backported".

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue32820\>


    @iritkatriel
    Copy link
    Member

    In that case, should this issue be closed?

    @ericvsmith
    Copy link
    Member

    At this point, it's a documentation-only issue. This new feature isn't documented.

    It might be less confusing to close this issue and open a new one. I'll do that shortly.

    @ericvsmith ericvsmith added the 3.10 only security fixes label Oct 17, 2020
    @ericvsmith
    Copy link
    Member

    I've created bpo-42061 for the documentation. Hopefully marking that issue as easy and newcomer friendly will attract some attention.

    Thanks ewosborne and Serhiy for adding this feature, and everyone for their input.

    @ewosborne
    Copy link
    Mannequin Author

    ewosborne mannequin commented Oct 17, 2020

    Did I really ship an enhancement without documenting it? I am a terrible
    person. I will pick up the issue and take care of it.

    eric

    On Sat, Oct 17, 2020 at 9:07 AM Eric V. Smith <report@bugs.python.org>
    wrote:

    Eric V. Smith <eric@trueblade.com> added the comment:

    I've created bpo-42061 for the documentation. Hopefully marking that
    issue as easy and newcomer friendly will attract some attention.

    Thanks ewosborne and Serhiy for adding this feature, and everyone for
    their input.

    ----------
    resolution: -> fixed
    stage: patch review -> resolved
    status: open -> closed


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue32820\>


    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.9 only security fixes 3.10 only security fixes docs Documentation in the Doc dir stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    7 participants