-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
Add and document __format__ method for IPv[46]Address #77001
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
Comments
This issue adds two things to the ipaddress lib:
|
test_not_an_index_issue15559() disallows __index__, so nevermind. Will resubmit without __index__ to get the full binary string, though. |
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' |
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}' |
Faster, too. Your way: I was a little worried about doing it all as a clever one-liner but this eric On Sun, Feb 11, 2018 at 6:21 PM Eric V. Smith <report@bugs.python.org>
|
The usefulness of this feature looks questionable to me. |
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? |
It is often useful to have non-decimal representations of IP addresses. In [23]: a In [24]: x = bin(int(a))[2:] In [25]: full_x = '0b' + ('0' * (32-len(x)) + x) In [26]: full_x Although, as Eric Smith has pointed out, there's a one liner way to do eric On Mon, Feb 12, 2018 at 7:39 AM Christian Heimes <report@bugs.python.org>
|
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. |
IPv6 is nasty no matter how you do it (cf. Binary's not about direct readabilty, but about ease of comparison. It's '0b00000001000000100000001100000100' and have them figure out whether the mask contains both hosts than to show '1.2.3.4' But I certainly agree that this is somewhat niche and a convenience eric On Mon, Feb 12, 2018 at 8:46 AM Serhiy Storchaka <report@bugs.python.org>
|
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:
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 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:
|
This is an interesting idea. I hacked together something to handle IPv4 Test output: IPv4 address 1.2.3.4 format: n format: x format: _b format: _n format: _x format: #b format: #n format: #x format: #_b format: #_n format: #_x Thanks! eric On Mon, Feb 12, 2018 at 9:15 PM Nick Coghlan <report@bugs.python.org> wrote:
|
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. |
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__(). |
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}") |
Cool, I will kick it over to python-ideas. I checked in some code to eric On Tue, Feb 13, 2018 at 11:56 PM Nick Coghlan <report@bugs.python.org>
|
I brought it up on python-ideas. Since I've not been through this process eric On Tue, Feb 13, 2018 at 11:56 PM Nick Coghlan <report@bugs.python.org>
|
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. |
I the bits() method is a dead end. The conclusion was to add __format__(). |
Yeah, bits() is dead. The thread has the same title, but I changed the PR I'll go over the code and clean it up. The docstring in particular is eric On Tue, Feb 20, 2018 at 3:35 AM Serhiy Storchaka <report@bugs.python.org>
|
And Eric, please avoid including the quote of previous message in your messages. This makes hard to read your messages and the whole discussion. |
I've updated the issue title to reflect the revised proposal (i.e. using __format__ rather than a new IP address specific method). |
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}') |
You always can use f'{addr!s:20}'. |
True enough. You'd think that I (of all people!) would know that. Nevermind. |
I dunno, I think this might be useful. A binary representation is itself a In [20]: a In [92]: f'{a}' In [21]: int(a) In [22]: f'{bin(int(a)):42s}' In [24]: f'{a:b}' In [25]: f'{a:b42s}' That last one should return '1000000100000001100000100 '. I was My current code supports [b, x, n] integer presentation types. I need to In [61]: f'{42:30}' In [62]: f'{int(a):30}' In [63]: f'{a:30}' In [66]: f'{a:42b}' Those last two seem odd. I think f'{a:30}' should return the same thing as In [69]: f'{str(a):30}' and f'{a:42b'} should return the same thing as this: In [77]: f'{bin(int(a)):42}' This also means supporting [>,<,^] alignment. And, of course, ignoring any In [86]: b In [87]: f'{b:6}' Thoughts? eric |
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
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' |
I have pushed out new diffs.
This was a decent-sized change, but all the tests pass so it looks OK to me. localized compile and import toplevel compile and import Is this worth trying to get clever about? It doesn't matter in my use case, eric |
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. |
PR 16378 simplifies the implementation. It also caches the compiled RE. Needed the documentation for the new feature. |
Do these PRs need to be backported? They're in 3.9 but not 3.8. |
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". |
Yes, I agree. Regardless of backport policy, this is a handy little eric On Fri, Oct 16, 2020 at 7:13 PM Eric V. Smith <report@bugs.python.org>
|
In that case, should this issue be closed? |
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. |
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. |
Did I really ship an enhancement without documenting it? I am a terrible eric On Sat, Oct 17, 2020 at 9:07 AM Eric V. Smith <report@bugs.python.org>
|
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: