API: Fix cython exception handling for exported extern C functions#22997
API: Fix cython exception handling for exported extern C functions#22997charris merged 3 commits intonumpy:mainfrom
Conversation
This hopefully fixes them for all the functions currently in the `.pyd` files. A surprising amount of them look like scary thing I wouldn't mind to just delete :). Anyway, had looked at some Cython code today and remembered this. Closes numpygh-19291
| int PyArray_CastTo (ndarray, ndarray) except -1 | ||
| int PyArray_CastAnyTo (ndarray, ndarray) except -1 | ||
| int PyArray_CanCastSafely (int, int) # writes errors | ||
| npy_bool PyArray_CanCastTo (dtype, dtype) # writes errors |
There was a problem hiding this comment.
I think all these "writes errors" should either be except * (if you can't say anything about the return value), or except? 0 (if errors always produce zero, but not all zeros are errors).
There was a problem hiding this comment.
Oh, "writes errors" here means it uses PyErr_WriteUnraiseable. So I think for cython that should be as good as nothing.
There was a problem hiding this comment.
Oh, I thought this meant "sets the exception but doesn't indicate it in the return value".
There was a problem hiding this comment.
PyErr_WriteUnraiseable -> PyErr_WriteUnraisable
Just to help searching for the documentation :)
numpy/__init__.cython-30.pxd
Outdated
| npy_intp PyArray_PyIntAsIntp (object) | ||
| int PyArray_Broadcast (broadcast) | ||
| int PyArray_Broadcast (broadcast) except -1 | ||
| void PyArray_FillObjectArray (ndarray, object) |
There was a problem hiding this comment.
Should this one (and probably a handful of other void functions) be except *? I assume this doesn't swallow errors.
There was a problem hiding this comment.
Arg, I didn't scan the void ones carefully, I guess. This isn't designed to error, but clearly can... (one of those bad functions). (the xincref/decref family is also weird, I think they tend to clear, but probably not always... and they shouldn't be used anway, especially in cython)
The incref/decref function shouldn't be able to fail (especially the decref). But right now they can, this will be fixed when we redo clearing (see numpygh-22924)
|
As confirmed by Stefan Behnel in the issue, I think this should be rather safe to do. Cython will insert error handling and any manual error handling simply becomes obsolete. |
|
Let's give this a shot. Thanks Sebastian. |
|
@scoder is it still relevant to push such changes to cython (in which case I can make a PR)? |
This hopefully fixes them for all the functions currently in the
.pydfiles. A surprising amount of them look like scary thing I wouldn't mind to just delete :).Anyway, had looked at some Cython code today and remembered this.
Closes gh-19291