MAINT: Ensure _convert_from_* functions set errors#15310
Conversation
95c85aa to
b572dc9
Compare
b572dc9 to
3ec6837
Compare
Previously some of these would return null without setting an error, meaning the caller had to clean up after them. Removes all direct callers of `PyArray_DescrConverter` in this file, since `_convert_from_any` has less indirection and a simpler calling convention.
3ec6837 to
4cd5a88
Compare
| PyArray_Descr *newdescr; | ||
| int ret = PyArray_DescrConverter(dtypedescr, &newdescr); | ||
|
|
||
| PyArray_Descr *newdescr = _convert_from_any(dtypedescr, 0); |
There was a problem hiding this comment.
I could make this respect alignment by adding an align argument to _try_convert_from_dtype_attr, but we'd then need to plumb that through PyArray_DescrFromTypeObject too, and the whole thing wouldn't make much sense anyway - user types are unlikely to have separate aligned and unaligned variants.
At any rate, this matches the old behavior.
There was a problem hiding this comment.
OK, lets keep old behaviours for now in any case. If you wanted, we could add a comment, or even issue. But my guess is that it is not worth the trouble right now.
| if (retcode == NPY_FAIL) { | ||
| res = NULL; | ||
| } | ||
| res = _convert_from_any(PyList_GET_ITEM(listobj, 0), align); |
There was a problem hiding this comment.
This fixes an extension of the bug in similar nonsensical code to #15287 (comment), namely
np.typeDict['mypoint'] = [('a', 'i1'), ('b', 'i2')]
assert np.dtype('mypoint,', align=True).isalignedstruct(xref gh-15296)
seberg
left a comment
There was a problem hiding this comment.
LGTM, I may have another look, but first check, cannot find any missing generic error setting or other issue.
| PyArray_Descr *newdescr; | ||
| int ret = PyArray_DescrConverter(dtypedescr, &newdescr); | ||
|
|
||
| PyArray_Descr *newdescr = _convert_from_any(dtypedescr, 0); |
There was a problem hiding this comment.
OK, lets keep old behaviours for now in any case. If you wanted, we could add a comment, or even issue. But my guess is that it is not worth the trouble right now.
|
Thanks @eric-wieser |
Previously some of these would return null without setting an error, meaning the caller had to clean up after them.
Removes all direct callers of
PyArray_DescrConverterin this file, since_convert_from_anyhas less indirection and a simpler calling convention.Follow-up work I'll file an issue for if this goes in - replace each call to
_report_generic_errorwith a useful error message. (done in #15316)