TST: convert remaining setup.py tests to meson instead #24206
Conversation
|
Windows builds are failing to find |
|
Yup, meson will need to implement support for setting a limited_api kwarg that:
|
| @@ -1,29 +0,0 @@ | |||
| """ | |||
There was a problem hiding this comment.
Here we don't use numpy.distutils, so this should continue to work unchanged. Hence it may be useful to keep around as an example? No need to test it for me, because it's so simple that it should be robust.
Although if you prefer to clean it up, also okay - no strong feelings about it. For the numpy/random I didn't think about this, but also that used numpy.distutils I think, so needed to go anyway.
| "limited_api", | ||
| sources=[os.path.join('.', "limited_api.c")], | ||
| include_dirs=[np.get_include()], | ||
| define_macros=macros, |
There was a problem hiding this comment.
This happens to work (perhaps, the test doesn't actually check that the extension is .abi3), but this seems wrong. setuptools.Extension expects a py_limited_api=True keyword: https://github.com/pypa/setuptools/blob/f7532b24c49b8285496734b544ea12a6fb6fdc52/setuptools/extension.py#L119.
|
Let's see how fast mesonbuild/meson#11745 to provide support for the limited API in meson progresses. |
|
PR mesonbuild/meson#11745 which provides a |
|
Where would we use the limited API? Seems relevant to be for OpenBLAS wheels, but not for NumPy. |
Only in tests/test_limited_api.py, I guess. ;) |
Exactly. Maybe that test is too simple, but it checks that one can call |
|
Thanks - that was obvious in hindsight - I was thinking this was a different PR.
I like the idea of removing our last test dependencies on The reason I'm a little hesitant is that the Meson change isn't a small/trivial diff, and it may make it more difficult to keep up with changes to the PR we actually need (the |
|
Rebased and refactored to only modify the cython example for now, and to leave the |
|
Hmm. Testing |
|
I removed the check, let's see which tests still need a pytest skip |
|
We only have a couple of CI jobs left that use |
|
cygwin is unhappy. If a test fails, it runs a "Check the extension modules on failure" CI step that is broken. One of the script's sed command is failing to get the module name (i.e. |
|
I'm not quite sure what is going wrong there. @DWesl maybe you can help, as the author of that |
|
Are you installing the import libraries on purpose? You shouldn't need any If that is on purpose, numpy/tools/list_numpy_dlls.sh Line 8 in a277f62 with an extra grep -F -v -e .dll.a, or changing the existing pattern to .dll$
|
We do not ship a wheel for cygwin, but apparently need to exclude some things when building the wheel in the CI build step. We do need a few import libraries to allow linking the various dlls that export I solved the immediate problem by getting the tests to pass, so the "on fail" build step is not executed. |
I remember the rounds of fixing the With those libraries in mind, I should look into fixing that regex. (EDIT: Done: #24570) |
The limited-api test has to wait for a new Meson version (see numpygh-24206). This converts the regular Cython test for `numpy.core`. [skip ci]
The limited-api test has to wait for a new Meson version (see numpygh-24206). This converts the regular Cython test for `numpy.core`. [skip ci]
I tried to remove
setuptoolsfromtest_requirements.txt, and found these tests were building c-extension modules using setup.py. I converted them to meson.We still need setuptools to pass the
numpy/tests/test_public_api.pytests. Some of the distutils modules fail to import if it is not found. I am leaving that for a further cleanup