gh-112014: correct buffer protocol support in ctypes (use native formats) - #155472
gh-112014: correct buffer protocol support in ctypes (use native formats)#155472skirpichev wants to merge 3 commits into
Conversation
…e formats) This allows better interoperation with the memoryview, e.g. support for ctypes arrays. Just like NumPy arrays, they also don't specify endianness: ```pycon >>> import numpy as np >>> memoryview(np.ndarray(3, dtype=np.float16)).format 'e' ``` But keep specified byteorder for byte-swapped types.
45bebeb to
feb9f5a
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
vstinner
left a comment
There was a problem hiding this comment.
I think that I can understand the rationale for this change, but it seems like @encukou has a different opinion on how the issue should be addressed: #112014 (comment).
| Correct the :ref:`buffer protocol <bufferobjects>` support in the | ||
| :mod:`ctypes` module to use the machine’s native format and byte order, | ||
| rather than explicitly specify endianness (by ``'<'`` or ``'>'``). The | ||
| later kept for byte-swapped types. Patch by Sergey B Kirpichev. |
There was a problem hiding this comment.
From this NEWS entry, it's uneasy to understand the rationale for the change. According to #112014 (comment), I understand that before, it was not possible to modify a ctypes type using memoryview, since the buffer format was rejected by memoryview when attempting to modify the view.
There was a problem hiding this comment.
it's uneasy to understand the rationale for the change
It's a bug :-) Memoryview support for ctypes buffers is a side effect of the bugfix.
Well, obviously it's a breaking change. We should decide whether the current behavior is a bug or not. My arguments are in the issue thread: #112014 (comment) |
There was a problem hiding this comment.
As is, currently, this PR is wrong.
I have a 64-bit machine where long has 8-bytes:
>>> import ctypes
>>> ctypes.sizeof(ctypes.c_long)
8
Per struct docs, an array of native longs should use the format l, but with this PR, it uses q:
>>> memoryview((ctypes.c_long*8)()).format
'q'
>>>
(Without this PR, it correctly uses <q, which means “8 bytes, little endian”.)
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
You are right, probably I should use Unfortunately, ctypes and struct use "slightly different format codes" (c). |
This allows better interoperation with the memoryview, e.g. support for ctypes arrays. Just like NumPy arrays, they also don't specify endianness:
But keep specified byteorder for byte-swapped types.