-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
BLD: switch to meson-python as the default build backend #23838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
bd85443
to
055345b
Compare
I think recent versions of rtools are preinstalled in GitHub actions. I suspect it is possible to split windows builds into separate cases for 32 and 64 bits and use the appropriate version of rtools. Of course, we could also just drop 32 bit builds for 2.0. |
This one:) xref gh-23717. |
Another option is to use MSVC on windows. We need 2 things from rtools: mingw (for gcc) and gfortran. We used to require mingw in order to mangle the openblas.a file to create an import library, but no longer need to do that, so mingw is no longer required. gfortran is needed for fortran, but that is only needed in the f2py tests. We could skip them on win32 (if gforfrtran is not found). |
A few different failures left:
Should not be too difficult to resolve, we need the right version from Rtools. @mattip will try to fix that up next week once he has access to his Windows machine.
The
This is due to the wheel being built against ILP64 OpenBLAS, and the symbol suffix handling for that needs wiring up. |
meson-python can handle setting up windows to use MSVC via a setup setting that will be ignored on non-windows.
I think that is also the path to adding 64-bit interfaces via a |
I will rebase this off main to clear the conflict and push a commit to add the |
hmm, maybe the |
16b0895
to
067e514
Compare
Getting somewhere. The manylinux wheel builds are failing with
The musllinux wheel builds have lots of DeprecationWarnings in tests, and fails MacOS wheels are failing some einsum tests
Windows wheels is failing to find OpenBLAS, the PyPy does not build, it seems the include paths are not right? |
This seems a bit too agressive, it's better to make it specific to cibuildwheel to avoid forcing MSVC usage on everyone who builds from source. Probably this is the way: [tool.cibuildwheel.windows]
config-settings = {"setup-args=--vsenv"} or alternatively as an env var:
|
I thought that was fixed for SciPy so it should work - good idea to disable that build config like you did and do that separately though.
That looks odd. Also there's a It'd be useful to make the cibuildwheel jobs non-verbose, the CI logs are hard to navigate now - the log is 23,900 lines long. It's the
I am unsure why other jobs are not failing on that, but the
So it's the test that needs tweaking here.
Those seem to be gone, at least I checked two jobs and it only shows the same |
I will reduce it
I will move the directive
I will try to fix that |
The wheels are building with |
@mattip do we want to default to |
Using
gives me
but the three tests still fail. I will commit what I have so far so others can try. I am not sure what is the delta to the other meson builds or to th distutil builds. There are also DeprecationWarnings |
There is still a problem with the license file. I think the syntax here
is different from the syntax in the meson-python test
|
Ah right, yes I had counted on PEP 639 moving forward by now. Let me have a look at this one right now. |
f52cb49
to
2670a13
Compare
Ah thanks, that explains it! The |
2670a13
to
df40e19
Compare
The 32-bit long double detection hack isn't quite working: |
df40e19
to
6f7d768
Compare
The macOS arm64 on Cirrus CI are still failing with:
That's related to forcing Other than that, this is getting close. Trying to get it green by tonight. |
Representative subset of warnings observed in CI: ``` f2py/tests/test_return_integer.py: 20 warnings /tmp/tmp.roLIRtEhPN/venv/lib/python3.9/site-packages/numpy/f2py/tests/test_return_integer.py:24: DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.) assert t(array([123], "d")) == 123 f2py/tests/test_return_real.py: 20 warnings /tmp/tmp.roLIRtEhPN/venv/lib/python3.9/site-packages/numpy/f2py/tests/test_return_real.py:23: DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.) assert abs(t(array([234])) - 234.0) <= err f2py/tests/test_return_real.py: 20 warnings /tmp/tmp.roLIRtEhPN/venv/lib/python3.9/site-packages/numpy/f2py/tests/test_return_real.py:24: DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.) assert abs(t(array([[234]])) - 234.0) <= err ```
This is way too slow, running a large part of the test suite twice. Issue 23975 tracks changing how this feature is tested.
Also remove a no longer needed `-std=c99` (this is already specified in the top-level `meson.build` file).
6f7d768
to
302f244
Compare
[skip circle] [skip cirrus]
Okay, this is ready to go in, phew:) All the follow-ups to this PR are listed in gh-23981. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, except for a few nits and checks for things that might need to be in the tracking issue
python3 -m pip install threadpoolctl && \ | ||
python3 tools/openblas_support.py --check_version" | ||
cd tools && \ | ||
python3 -m pytest --pyargs numpy" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has removed the check for the openblas version. Is that on purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we now have proper logging in the build log for the version, and this cannot really be wrong (no other OpenBLAS version is installed in CI). This job was painful to debug, so I trimmed it down as much as possible.
This is still green after merging in the I'll go ahead and merge this to keep the ball rolling. Thanks for the reviews and help @mattip! |
Time to give this a go now we're developing for 2.0 on
main
. Will also make it easier to use NumPymain
with Python 3.12 dev builds.Follows up on #23808 (comment).