API: Fix compat header and add new import helpers#25866
API: Fix compat header and add new import helpers#25866ngoldbaum merged 2 commits intonumpy:mainfrom
Conversation
bfc333e to
b75a069
Compare
The current import functions are a bit silly as they are all macros which immediately return on error. Thus we really should have a new helper anyway. The one added also checks for whether the API was already imported so that downstream can add it judiciously in case the proper setup is more difficult.
b75a069 to
fa028cf
Compare
fa028cf to
89be919
Compare
|
I like this just to have an idempotent function to import the API tables, LGTM. I'll wait to merge to give scipy folks a chance to look at this. |
What do you mean by backported? |
There was a problem hiding this comment.
I checked the following scenarios:
- unmodified SciPy
mainalongside this branch of NumPy for both building and testing: full suite passes - SciPy
mainwith the newnpy_2_compat.hfrom this PR vendored in along with the includes I used in scipy/scipy#20108 (but without the->fdescr changes in SciPy or NumPy), using NumPy1.26.4for both build and test: full suite passes - SciPy
mainwith those same changes/includes, but built against this NumPy PR and tested against NumPy1.26.4: full suite passes - SciPy
main+ same changes/includes, build and test against this PR branch: full suite passes
I didn't go as far as trying to revert the descr ->f reversion in gh-25862, because some of the same files are touched and may require deep expertise to resolve the conflicts the intended way.
I meant vendoring by users if they need new things (most of course shouldn't but arrfuns or user dtypes requires need it; arrfuns should not be super common since get/setitem have existing macros). |
|
Is there any doubt about adding the new idempotent (and actual) inline functions for importing? Because if this is delayed due to that, I would just take it out, even if I think it is useful. |
|
I think we're just low on reviewer bandwidth at the moment. I was sick on Friday and tried to take it easy this weekend so haven't had a chance to take a look at this again until now. Let's pull this in, we can iterate on the new functions if people object later. |
This PR does three somewhat related things:
npy_2_compat.hso now it should work fine when backported.import_arrayworks is odd, as it includes thereturnstatement, which IMO is terrible and misleading. I also wanted to tell downstream they can just import array judiciously without the need to figure out importing it exactly once at import time (which clearly nicer, but may not be trivial to update in all cases).Thus I added
PyArray_ImportNumPyAPIandPyUFunc_ImportUFuncAPIas functions, the first is back-portable since otherwise I can't suggest to use it :).I will double check the new pattern works for backporting (I assume it does, but marking as draft until I checked).I believe this should alleviate the main problem with the
arrfuncsPR, although it may be that theGETITEMandSETITEMmacros need to move headers still.