Repository navigation
BUG: Fix setitem on UNICODE, STRING, and LONGDOUBLE - #8903
Conversation
This addresses some of numpy#8902, preventing precision loss
This seemed arbitrary, and was not implemented consistently. Also make the error messages match When combined with the last two commits, this fixes numpy#8902
| if (!PyBytes_Check(op) && !PyUnicode_Check(op) && | ||
| PySequence_Check(op) && PySequence_Size(op) > 0) { | ||
| /* Handle case of assigning from an array scalar */ | ||
| if (PyArray_Check(op) && PyArray_NDIM((PyArrayObject *)op) == 0) { |
There was a problem hiding this comment.
this is PyArray_IsZeroDim
There was a problem hiding this comment.
as this is copied on longdouble, it might make sense to move this into a function with the setitem function as argument
There was a problem hiding this comment.
Yep, this exists in 3 places, all of them identical.
My worry was that wrapping this in a function would give the method authenticity - elsewhere in this file, we use a different route for converting 0d arrays to scalars.
There was a problem hiding this comment.
So, here we use
PyArray_ToScalar(PyArray_BYTES((PyArrayObject *)op),
(PyArrayObject *)op);
But in cdouble, we have
op2 = PyArray_DESCR((PyArrayObject *)op)->f->getitem(
PyArray_BYTES((PyArrayObject *)op),
(PyArrayObject *)op);
Are these different in any important ways?
There was a problem hiding this comment.
no clue, apparently both work. Might be worth testing using getitem in the other 3 cases and see if something fails.
As it already are two approaches in the file imo it is enough to move the copied one into a function and add a TODO
There was a problem hiding this comment.
Switched to PyArray_IsZeroDim. The way this is currently written (recursing) doesn't lend itself particularly well to being placed in a function.
And recursing is actually a pretty tidy way to deal with the refcounting here - the alternative being replacing every early return with a goto fail;
There was a problem hiding this comment.
I pushed a commit moving it into a function? what does not lend itself to it?
the return is a little odd in the flow of the function but it is the same without the function.
There was a problem hiding this comment.
I guess I was a little reluctant to pass a function pointer, but I don't have a good justification for that. Looks good to me now, anyway, so thanks
3bcbbcf to
94a376f
Compare
9048a9f to
c5f62da
Compare
|
thanks |
This was not caught in numpygh-8903
This was not caught in numpygh-8903
See #8902