Skip to content

gh-112014: correct buffer protocol support in ctypes (use native formats) - #155472

Closed
skirpichev wants to merge 3 commits into
python:mainfrom
skirpichev:use-native-endianness-in-ctypes/112014
Closed

gh-112014: correct buffer protocol support in ctypes (use native formats)#155472
skirpichev wants to merge 3 commits into
python:mainfrom
skirpichev:use-native-endianness-in-ctypes/112014

Conversation

@skirpichev

@skirpichev skirpichev commented Aug 10, 2026

Copy link
Copy Markdown
Member

This allows better interoperation with the memoryview, e.g. support for ctypes arrays. Just like NumPy arrays, they also don't specify endianness:

>>> import numpy as np
>>> memoryview(np.ndarray(3, dtype=np.float16)).format
'e'

But keep specified byteorder for byte-swapped types.

…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.
@skirpichev
skirpichev force-pushed the use-native-endianness-in-ctypes/112014 branch from 45bebeb to feb9f5a Compare August 10, 2026 07:55
@skirpichev

This comment was marked as resolved.

@bedevere-bot

This comment was marked as resolved.

@skirpichev
skirpichev marked this pull request as ready for review August 10, 2026 08:53
@skirpichev
skirpichev requested review from encukou and vstinner and removed request for encukou August 10, 2026 08:53

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@skirpichev

Copy link
Copy Markdown
Member Author

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).

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)

@encukou encukou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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”.)

@bedevere-app

bedevere-app Bot commented Aug 11, 2026

Copy link
Copy Markdown

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@skirpichev
skirpichev marked this pull request as draft August 11, 2026 07:25
@skirpichev

Copy link
Copy Markdown
Member Author

You are right, probably I should use _ctypes_alloc_format_string_for_type() helper for swapped types. And just copy format code per default.

Unfortunately, ctypes and struct use "slightly different format codes" (c).

@skirpichev
skirpichev marked this pull request as ready for review August 12, 2026 03:26
@skirpichev

Copy link
Copy Markdown
Member Author

@encukou, this probably needs some more work to adjust and/or add tests. But I think it's ready for review as a concept.

@skirpichev

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again.

@bedevere-app

bedevere-app Bot commented Aug 12, 2026

Copy link
Copy Markdown

Thanks for making the requested changes!

@encukou: please review the changes made to this pull request.

@bedevere-app
bedevere-app Bot requested a review from encukou August 12, 2026 06:02
@encukou

encukou commented Aug 12, 2026

Copy link
Copy Markdown
Member

OK, I'll review.

As I said in the issue: in anything that can be used for storage/interchange, I'd prefer to keep describing data using explicit sizes rather than platform-specific types.

While there are use cases where this change would help, it makes things less explicit. I don't think it is worth the behaviour change.

Feel free to discuss the concept in the issue.

@encukou encukou closed this Aug 12, 2026
@skirpichev
skirpichev deleted the use-native-endianness-in-ctypes/112014 branch August 12, 2026 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants