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

PEP 688 buffer methods #3148

Open
davidhewitt opened this issue May 9, 2023 · 4 comments
Open

PEP 688 buffer methods #3148

davidhewitt opened this issue May 9, 2023 · 4 comments
Milestone

Comments

@davidhewitt
Copy link
Member

PEP 688 defines new methods __buffer__ and __release_buffer__ with signatures as follows:

    def __buffer__(self, flags: int, /) -> memoryview: ...
    def __release_buffer__(self, buffer: memoryview, /) -> None: ...

At the moment we have __getbuffer__ and __releasebuffer__ which have slightly different signatures, because they deal in the ffi::Py_buffer type.

It would be nice if we could support the PEP 688 methods, ideally with a way to use the full ffi::Py_buffer flexibility when users need it.

One option could be that we only support the PEP 688 way as first-class, and then allow the user to annotate a method with #[pyo3(unsafe_type_slot = bf_getbuffer)] to directly insert a method into the buffer slot (this could be generally useful for all type slots without requiring us to write any special machinery).

@JelleZijlstra
Copy link

I wrote PEP 688 and I'd be happy to answer any questions you may have.

In the CPython implementation I had to be very careful in what kind of calls to bf_releasebuffer are allowed. Naively, you'd just have the Python __release_buffer__ method call the bf_releasebuffer slot function, but that can easily break invariants in the C code for buffer classes if users call .__release_buffer__() with bogus arguments. So everything is set up so that you can only call bf_releasebuffer with a buffer you got back from bf_getbuffer yourself. Presumably you'll need a similar mechanism for protecting buffer classes created in Rust.

@davidhewitt
Copy link
Member Author

Thanks @JelleZijlstra. Do you wrap the bf_releasebuffer slot for heap types? If so, we might automatically benefit from your work if we just translate __buffer__ and __release_buffer__ methods into a bf_releasebuffer implementation.

@JelleZijlstra
Copy link

Any call to .__release_buffer__() on an extension type will go through wrap_releasebuffer
(https://github.com/python/cpython/blob/ce558e69d4087dd3653207de78345fbb8a2c7835/Objects/typeobject.c#L8192), which is supposed to protect against incorrect calls (with an unrelated or already-released buffer).

@davidhewitt
Copy link
Member Author

Great, in which case that should apply to us, so the action here for us is just to replace our current #[pymethods] which implement the buffer protocol __getbuffer__ and __releasebuffer__ with the new PEP 688 names. (That's purely in our macro implementation.)

@davidhewitt davidhewitt mentioned this issue Jun 16, 2023
6 tasks
@davidhewitt davidhewitt added this to the 0.20 milestone Aug 11, 2023
@davidhewitt davidhewitt modified the milestones: 0.20, 0.21 Oct 2, 2023
@davidhewitt davidhewitt modified the milestones: 0.21, 0.22 Mar 29, 2024
@davidhewitt davidhewitt modified the milestones: 0.22, 0.24 Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants