BUG: make void-scalar getfield/setfield use ndarray methods #5947
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a continuation of #5642.
This PR removes most of the logic in
voidtype_getfield
andvoidtype_setfield
. Instead they simply call the ndarraygetfield
andsetfield
. This solves bugs related to void-scalar assignment, #3126 and #3561. It also makes these functions safer since ndarray's get/setfield does safety checks to avoid segfaults involving object arrays.Additional minor notes:
I changed the calling convention of
voidtype_getfield
. Previously it accepted(dtype, offset, title)
and dropped the (optional) title. Now it expects only(dtype, offset)
, just like ndarray getfield. This simplifies thevoidtype_getfield
code.I also removed code that does byteswapping in both getfield and setfield. After looking at it for a while, I am pretty convinced this code is unnecessary since in both cases we use
gentype_generic_method
to convert the void scalar to a 0-d ndarray, do the get/setfield, and then convert the returned value to a scalar usingPyArray_ToScalar
(inPyArray_Return
).PyArray_ToScalar
already does the byteswap for us. Therefore, any scalars that reachvoidtype_getfield
are already in NBO and there is no need to swap again.Also, at the end of
voidtype_setfield
I effectively doarr[()] = val
, and actually create an empty tuple object. Is that really the best way to use setfield, given that arr is possible 0-d?