-
Notifications
You must be signed in to change notification settings - Fork 925
add PyUntypedBuffer
#5458
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
base: main
Are you sure you want to change the base?
add PyUntypedBuffer
#5458
Conversation
|
Is anyone interested in reviewing this? If not, I am tempted to merge this soon to move this and #5245 forward. |
src/buffer.rs
Outdated
| #[inline] | ||
| pub fn buf_ptr(&self) -> *mut c_void { | ||
| self.raw().buf | ||
| self.0.buf_ptr() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we do not necessarily need to have these forwarding methods. They should be available via Deref directly from the underlying PyUntypedBuffer. It would be a breaking change however because fully qualified syntax would change,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question, and I'm open to not having them. I added exactly to avoid breaking fully-qualified syntax, though I don't know how much that'll really be used in practice.
If we removed, I guess we could just nod to this change in the migration guide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I would not expect it to be a problem in practice. Outside of macros fully-qualified syntax is rarely used and migrating would be trivial anyway. I think I would remove them, but I don't feel super strong about that and you can also decide to keep if you prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed these, we can restore them if we have complaints.
This is a proposal to make a first step towards resolving #5245
The idea is to add
PyUntypedBuffer, which is a sibling toPyBuffer<T>which does not care about the actual element type it holds inside. I got the idea of the name fromPyUntypedArrayinrust-numpy.I can imagine that this is potentially applicable in a number of cases where the user wants to attempt to get a buffer export from an object but doesn't necessarily know what element type the object will export.
PyUntypedBuffer::get()can be used to get the export,.format()can then be inspected to deduce what concrete elementTto cast the buffer to.I think while we're here I moved a lot of the implementations of methods from
PyBuffer<T>toPyUntypedBuffer, which may slightly reduce generic code bloat.