TST: convert remaining setup.py tests to meson instead #24206

mattip · 2023-07-18T08:36:43Z

I tried to remove setuptools from test_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.py tests. Some of the distutils modules fail to import if it is not found. I am leaving that for a further cleanup

mattip · 2023-07-18T10:46:59Z

Windows builds are failing to find python3.lib when building with the limited API. Is this a known problem with meson?

"link"  /MACHINE:x64 /OUT:limited_api.cp311-win_amd64.pyd limited_api.cp311-win_amd64.pyd.p/45439c8367f143d67c47043d0b62b0da35190854_core_tests_examples_limited_api_limited_api.c.obj "/nologo" "/release" "/nologo" "/OPT:REF" "/DLL" "/IMPLIB:limited_api.cp311-win_amd64.lib" "C:\hostedtoolcache\windows\Python\3.11.4\x64\libs\python311.lib" "kernel32.lib" "user32.lib" "gdi32.lib" "winspool.lib" "shell32.lib" "ole32.lib" "oleaut32.lib" "uuid.lib" "comdlg32.lib" "advapi32.lib"

LINK : fatal error LNK1104: cannot open file 'python3.lib'
```

eli-schwartz · 2023-07-18T22:56:13Z

Yup, meson will need to implement support for setting a limited_api kwarg that:

defines the macro for you
makes sure to link to the right python DLL
adds the correct .abi3 extension

rgommers

Thanks @mattip. Not touching the limited API test for now and updating the rest sounds good to me.

numpy/core/tests/examples/cython/meson.build

rgommers · 2023-07-20T09:34:35Z

numpy/core/tests/examples/cython/setup.py

@@ -1,29 +0,0 @@
-"""


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.

rgommers · 2023-07-20T09:52:05Z

numpy/core/tests/examples/limited_api/setup.py

-    "limited_api",
-    sources=[os.path.join('.', "limited_api.c")],
-    include_dirs=[np.get_include()],
-    define_macros=macros,


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.

mattip · 2023-07-21T06:53:41Z

Let's see how fast mesonbuild/meson#11745 to provide support for the limited API in meson progresses.

mattip · 2023-08-27T14:57:55Z

PR mesonbuild/meson#11745 which provides a python.allow_limited_api option. If set to true meson will build a limited API c-extension where possible (not on PyPy). It is targetted for a future meson 1.13 release. Should we merge it to our vendored meson code for tests?

rgommers · 2023-08-27T18:01:07Z

Where would we use the limited API? Seems relevant to be for OpenBLAS wheels, but not for NumPy.

eli-schwartz · 2023-08-27T18:11:13Z

Where would we use the limited API?

Only in tests/test_limited_api.py, I guess. ;)

mattip · 2023-08-27T18:57:01Z

in tests/test_limited_api.py

Exactly. Maybe that test is too simple, but it checks that one can call import_array(); import_umath(); in a module compiled for the limited API. It came from #20818 in response to a regression identified in issue #13784, where our headers exposed non-limited-API C-API calls.

rgommers · 2023-08-28T07:49:00Z

Thanks - that was obvious in hindsight - I was thinking this was a different PR.

Should we merge it to our vendored meson code for tests?

I like the idea of removing our last test dependencies on setuptools in principle, but this doesn't seem urgent. How about making the test only run if setuptools is installed, and otherwise skipping it?

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 features module for SIMD support). And there's actually nothing wrong with the current test - code like that needs to stay working too.

mattip · 2023-08-28T10:51:09Z

Rebased and refactored to only modify the cython example for now, and to leave the setup.py file for reference.

mattip · 2023-08-28T11:45:19Z

Hmm. Testing setuptools imports it, but we have pinned to an older version. So the import hits a DeprecationWarning. It was debugged in scipy that this is due to pinning setuptools<62 together with python3.11.

mattip · 2023-08-28T11:47:21Z

I removed the check, let's see which tests still need a pytest skip

rgommers · 2023-08-28T12:18:15Z

We only have a couple of CI jobs left that use setup.py; we should be able to unpin very soon (if not now).

mattip · 2023-08-28T14:06:58Z

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. _multiarray_tests) from the dll (i.e. /usr/local/lib/python3.9/site-packages/numpy/core/_multiarray_tests.cpython-39-x86_64-cygwin.dll.a

rgommers · 2023-08-28T15:19:48Z

I'm not quite sure what is going wrong there. @DWesl maybe you can help, as the author of that sed script?

DWesl · 2023-08-28T16:52:54Z

Are you installing the import libraries on purpose? You shouldn't need any .dll.a files unless you want to link against them, and there's not too many reasons to link against a hidden test extension module.

If that is on purpose, tools/list_numpy_dlls.sh needs to be modified to exclude those files, which is easy enough to do here:

numpy/tools/list_numpy_dlls.sh

Line 8 in a277f62

grep -F .dll); do echo ${site_packages}/${name}; done`

with an extra grep -F -v -e .dll.a, or changing the existing pattern to .dll$

mattip · 2023-08-28T20:17:19Z

Are you installing the import libraries on purpose?

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 numpy.random c-external functions.

I solved the immediate problem by getting the tests to pass, so the "on fail" build step is not executed.

DWesl · 2023-08-28T21:07:41Z

Are you installing the import libraries on purpose?

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 numpy.random c-external functions.

I remember the rounds of fixing the numpy.random c-extensions so they work with both upstream Cython and the distribution Cython.

With those libraries in mind, I should look into fixing that regex. (EDIT: Done: #24570)

[skip ci]

rgommers

This LGTM now and CI is all green, so in it goes. Thanks @mattip & reviewers!

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]

github-actions bot added the 05 - Testing label Jul 18, 2023

mattip mentioned this pull request Jul 18, 2023

When building a python c-extension using the limited_api, windows should link with python3.lib mesonbuild/meson#12008

Closed

rgommers reviewed Jul 20, 2023

View reviewed changes

mattip mentioned this pull request Jul 21, 2023

BUG: importing numpy.distutils replaces the global logger with a custom class which doesn't support kwargs #24213

Closed

charris added the 09 - Backport-Candidate PRs tagged should be backported label Jul 21, 2023

charris added this to the 1.26.0 release milestone Jul 25, 2023

mattip added 2 commits August 28, 2023 13:30

TST: convert test_limited_api to meson

9b50ad4

TST: skip limited-api test if setuptools not found

978af4a

mattip force-pushed the more-meson branch from 506b280 to 978af4a Compare August 28, 2023 10:50

TST: remove check that triggers deprecation warning in old setuptools

cbbae77

TST: skip test if meson is not installed

091cba6

DWesl mentioned this pull request Aug 28, 2023

CI: Exclude import libraries from list of DLLs on Cygwin. #24570

Merged

charris mentioned this pull request Aug 30, 2023

CI: Exclude import libraries from list of DLLs on Cygwin. #24590

Merged

STY: clean up spurious print statement and blank line

2f1e4de

[skip ci]

rgommers approved these changes Aug 31, 2023

View reviewed changes

rgommers merged commit 1195ff5 into numpy:main Aug 31, 2023
2 checks passed

charris mentioned this pull request Sep 1, 2023

TST: convert cython test from setup.py to meson #24616

Merged

charris removed the 09 - Backport-Candidate PRs tagged should be backported label Sep 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TST: convert remaining setup.py tests to meson instead #24206

TST: convert remaining setup.py tests to meson instead #24206

mattip commented Jul 18, 2023 •

edited

mattip commented Jul 18, 2023

eli-schwartz commented Jul 18, 2023

rgommers left a comment

rgommers Jul 20, 2023

rgommers Jul 20, 2023

mattip commented Jul 21, 2023

mattip commented Aug 27, 2023

rgommers commented Aug 27, 2023

eli-schwartz commented Aug 27, 2023

mattip commented Aug 27, 2023

rgommers commented Aug 28, 2023

mattip commented Aug 28, 2023

mattip commented Aug 28, 2023

mattip commented Aug 28, 2023

rgommers commented Aug 28, 2023

mattip commented Aug 28, 2023

rgommers commented Aug 28, 2023

DWesl commented Aug 28, 2023 •

edited

mattip commented Aug 28, 2023

DWesl commented Aug 28, 2023 •

edited

rgommers left a comment

TST: convert remaining setup.py tests to meson instead #24206

TST: convert remaining setup.py tests to meson instead #24206

Conversation

mattip commented Jul 18, 2023 • edited

mattip commented Jul 18, 2023

eli-schwartz commented Jul 18, 2023

rgommers left a comment

Choose a reason for hiding this comment

rgommers Jul 20, 2023

Choose a reason for hiding this comment

rgommers Jul 20, 2023

Choose a reason for hiding this comment

mattip commented Jul 21, 2023

mattip commented Aug 27, 2023

rgommers commented Aug 27, 2023

eli-schwartz commented Aug 27, 2023

mattip commented Aug 27, 2023

rgommers commented Aug 28, 2023

mattip commented Aug 28, 2023

mattip commented Aug 28, 2023

mattip commented Aug 28, 2023

rgommers commented Aug 28, 2023

mattip commented Aug 28, 2023

rgommers commented Aug 28, 2023

DWesl commented Aug 28, 2023 • edited

mattip commented Aug 28, 2023

DWesl commented Aug 28, 2023 • edited

rgommers left a comment

Choose a reason for hiding this comment

mattip commented Jul 18, 2023 •

edited

DWesl commented Aug 28, 2023 •

edited

DWesl commented Aug 28, 2023 •

edited