-
-
Notifications
You must be signed in to change notification settings - Fork 30k
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 default arguments for int.to_bytes() #89318
Comments
In the PEP-467 discussion, I proposed being able to use >>> (65).to_bytes()
b'A' IOW, adding default arguments for the It occurs to me that this is (1) useful on its own merits; (2) easy to do. So I've done it. Creating this bug so I can link a PR against it. |
Rather than defaulting to sys.byteorder, could the byte order default to None and only be optional when not needed? (input value fits in a single byte, output is a single byte) Otherwise the difference in defaults between this method and the struct module (network byte order rather than host byte order) could be very confusing. |
Never mind, I've forced network byte order in struct strings for so long I had forgotten that native byte order was also the default there. Hence I withdraw that objection. |
So (128).to_bytes() will raise an error, right? I afraid also that it will lead to some programs working correctly only on platforms with the most common byte order, just because authors are not aware of byte ordering. Currently the interface forces programmers to read something about byte ordering. |
Ah, signed=False by default, so (128).to_bytes() will work. But I still worry that it can provoke writing more errorprone code. |
Can you elaborate on that? Obviously no existing code will change behavior. I really don’t expect people to write |
Perhaps instead of the system byte ordering, choose one for the default so that default encoding/decoding will work cross platform. I think "little" is the most common (intel and arm). |
For the common case where you’re using all defaults, it won’t matter. byteorder doesn’t matter when length=1.
|
Serhiy is likely thinking of other the other cases. Prior to this discussion, the principal use for to_bytes and from_bytes was for integers larger than a single byte. If we're going to add a *byteorder* default, it needs to make sense for those cases as well. |
Raymond, please don't do this. We already have a "sensible default" in a network context, and it is big endian. Having another "sensible default" opposite to the previous one is really no way to ensure interoperability. (https://xkcd.com/927/ only becomes more ridiculous when the number in question is 2.:) I don't want to think about whether the way machines A and B exchange data can be called "a network" or not. Of course, having the byteorder optional when there's only one (unsigned) byte is good. |
I'd also really like to avoid a system-dependent default. The danger is that code of the form some_externally_supplied_integer.to_bytes(length=4) can be written and thoroughly tested, only to fail unexpectedly some time later when that code happens to meet a big-endian machine. In most real-world cases with input length >= 1, it's unlikely that the system byteorder is the right thing, especially for from_bytes: what you need to know is what endianness the integer was encoded with, and that's not likely to be well correlated with the endianness that the machine you're running on right now happens to be using. (The choice of default obviously doesn't matter in cases where you're encoding and decoding on the same system, but there are going to be plenty of cases where that's not true.) This is essentially the same issue that PEP-597 starts to address with |
Exactly, a platform-dependent default is a bad idea. A default allows using the function without the code author & reviewer even being *aware* that there is a choice, and that is dangerous. |
I dislike the idea of adding a default length to int.to_bytes(). The length changes the meaning of the output: >>> (1).to_bytes(2, 'big')
b'\x00\x01'
>>> (1).to_bytes(1, 'big')
b'\x01' If the intent is to "magically cast an integer to a byte strings", having a fixed length of 1 doesn't help: >>> (1000).to_bytes(1, "big")
OverflowError: int too big to convert If the intent is to create a bytes string of length 1, I'm not sure that "re-using" this existing API for that is a good idea. |
Just to point out, struct module also uses “native” (i.e. system) byte order by default. Any choice other than that for to_bytes() seems both arbitrary and inconsistent.
|
On Sep 10, 2021, at 04:06, STINNER Victor <report@bugs.python.org> wrote:
Why not? It seems an obvious and simple convenience. |
Petr Viktorin <encukou@gmail.com> added the comment:
I’m not convinced. I’m more concerned with the obscurity of the API. If I saw its use in some code I was reviewing, I’d look it up, and then I’d know exactly what it was doing. |
[Mark Dickinson]
[Petr Viktorin]
I concur with Petr and Mark. The principal use case for int.to_bytes is to convert an integer of arbitrary size to a binary format for storage or transmission. The corresponding file load or received data needs to symmetrically restore the int. The default (whether little or big) needs to be the same on both sides to prevent bugs. Otherwise, for portable code, we would have to recommend that people not use the default because the output isn't deterministic across systems. By way of comparison, we've had long standing issues like this in other parts of the language. For example, this gives inconsistent encodings across systems: with open(filename) as f:
f.write(text) Not long ago, Inada had to sweep through and add encoding="utf-8" to fix all the bugs caused by the default platform dependent encoding. Arguably, most code that has ever been written without an explicit encoding is wrong if the files were intended to be shared outside the local file system. So if Veky wants the default to be "big", that's fine by me. The important thing is that a *consistent* default be used (not platform dependent). I won't argue for a "sensible default" because apparently Veky has different sensibilities. |
My sensibilities are irrelevant here. I'm just saying we already have a standard byte order for data in transit, and it was introduced long before this thing called internet (it was with capital I back then:) started to interest me. |
The struct module has 4 different modes. By default it uses not only native byte order, but native sizes and alignments which depend on OS and compiler. You need to know all these details just to understand the format codes. I think that the struct module user is more prepared to work with different word sizes and therefore with different byte ordering. |
In the stdlib, there is only one use of to_bytes() with sys.byteorder (2 in tests), 16 uses of to_bytes()/from_bytes() with 'little' (22 in tests) and 22 uses with 'big' (33 in tests). So making sys.byteorder the default will help almost nobody, and the advantage of 'big' over 'litte' is so small, that making any of them the default may only help in a half of cases and confuse in the other half. |
Interestingly, "little" is faster than "big". $ python3.10 -m timeit -r11 -s 'x=3452452454524' 'x.to_bytes(10, "little")'
5000000 loops, best of 11: 82.7 nsec per loop
$ python3.10 -m timeit -r11 -s 'x=3452452454524' 'x.to_bytes(10, "big")'
5000000 loops, best of 11: 90.6 nsec per loop |
Perhaps it is because "little" is checked first. One call of _PyUnicode_EqualToASCIIId() for "little" and two for "big". |
I know you would. But there are many others who just try things until they work. Also, if this does become *the* way to create bytes, it won't be obscure any more -- but you'd still need to remember to always specify byteorder for length > 1. That is, unless you *want* platform-specific behavior, which I don't think is all that often. Even in this case, you want to think about the issue, and omitting the argument is a bad way to encode that you thought about it. --- Hm, what happened to the idea of only requiring byteorder for |
That’s okay, Brandt’s improved sys.byteorder is fastest <wink>. % ./python.exe -m timeit -r11 -s 'x=3452452454524' 'x.to_bytes(10, "little")' On Sep 12, 2021, at 04:20, Raymond Hettinger <report@bugs.python.org> wrote:
|
I created a Discourse poll: https://discuss.python.org/t/what-should-be-the-default-value-for-int-to-bytes-byteorder/10616 |
I'd probably say "In the face of ambiguity, refuse the temptation to guess". As there's disagreement about the 'correct' default, make it None and require either "big" or "little" if length > 1 (the default). |
>>> (65).to_bytes()
b'A' It seems like your proposal is mostly guided by: convert an int to a byte (bytes string of length 1). IMO this case is special enough to justify the usage of a different function. What if people expect int.to_bytes() always return a single byte, but then get two bytes by mistake? ch = 256
byte = ch.to_bytes()
assert len(byte) == 2 # oops A function dedicated to create a single byte is expected to raise a ValueError for values outside the range [0; 255]. Like: >>> struct.pack('B', 255)
b'\xff'
>>> struct.pack('B', 256)
struct.error: ubyte format requires 0 <= number <= 255 |
Since this PEP is controversial, and this issue seems to be controversial as well, maybe this idea should be part of the PEP. |
The poll is invalid, since the option that most people want is deliberately not offered. |
Just reread the thread. AFAICT not a single use case was presented for having system byte ordering as the default. However, multiple respondents have pointed out that a default to system byte ordering is a bug waiting to happen, almost ensuring that some users will encounter unexpected behaviors when crossing platforms. We've seen issues like that before and should avoid them. We don't really need a poll. What is needed is for the system byte ordering proponents to present valid reasons why it would useful and to address the concerns that it is actually harmful. If the proposal goes through despite the concerns, we should ask folks writing lint tools to flag every use of the default as a potential bug and advise people to never use the default unless they know for sure that it is encoding only a single byte. Personally, I would never let system byte ordering pass a code review. |
I wonder whether there should be a couple of other endianness values, namely, "native" and "network", for those cases where you want to be explicit about it. If you use "big" it's not clear whether that's because you want network endianness or because the platform is big-endian. |
Jelle suggested that over in Discourse, and I’m not opposed, but it does mean that there’s no natural default for byteorder in int.from_bytes(). |
On Sep 13, 2021, at 13:38, STINNER Victor <report@bugs.python.org> wrote:
Like bchr() ? <wink>
The OverflowError you’ll get seems reasonable. |
On Sep 13, 2021, at 13:39, Vedran Čačić <report@bugs.python.org> wrote:
*Is* there an option that most people want? |
I'd say yes. Of course, one way to ascertain that would be to conduct a valid pool. ;-) |
On Sep 13, 2021, at 22:12, Vedran Čačić <report@bugs.python.org> wrote:
People can always comment otherwise in the Discourse thread. |
"big" by default |
The commit title is wrong, the default "big" not sys.byteorder: int.to_bytes(length=1, byteorder='big', *, signed=False)
int.from_bytes(bytes, byteorder='big', *, signed=False) |
On Sep 16, 2021, at 00:36, STINNER Victor <report@bugs.python.org> wrote:
Oops |
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: