BUILD/CI Switch to Meson as main build backend #28506
Conversation
|
The CI failures are in |
|
I like the settings we have with the editable install now (on meson), so I agree with your points. As for |
|
I also have the feeling that the concerns I had with meson-python editable installs are all being addressed. Note that we will also need to check that the CD pipeline works now that meson is the default build in the pyproject.toml. But since we don't plan to make a release in the coming weeks, we have the time to merge and check that the nightly builds work as expected. Question: any impact on the conda-forge build? |
|
Also there are now conflicts on the lock file so this PR needs an update. |
…nto meson-main-build-backend
I opened a PR: conda-forge/scikit-learn-feedstock#250, let's see what conda-forge CI has to say about it! |
After a few iterations, the conda-forge feedstock PR is green. |
…nto meson-main-build-backend
…nto meson-main-build-backend
|
The consensus during our call today is that this is OK to be merged in As I mentioned during our call today, it would be great if someone had a closer look at the I commented a bit on the motivation behind the changes in #28506 (comment) |
…nto meson-main-build-backend
| @@ -24,9 +24,7 @@ inplace: | |||
| $(PYTHON) setup.py build_ext -i | |||
There was a problem hiding this comment.
Maybe add a comment to explain that this is only useful when building with setuptools, e.g. after having run python setup.py develop or by setting PYTHONPATH manually.
Maybe we could have a dev-setuptools entry to make that more explicit?
Not sure about the latter.
There was a problem hiding this comment.
Could you add a new --update-pyproject option to the sklearn/_min_dependencies.py __main__ script to update the various dependency groups in pyproject.toml from the tag_to_packages mapping?
Otherwise we will have to maintain this mapping by hand and I am pretty sure we will forget things and botch package metadata in releases as a result.
Other than that, LGTM!
pyproject.toml
Outdated
| "numpydoc>=1.2.0", | ||
| "pooch>=1.6.0", | ||
| ] | ||
| maintenance = ["conda-lock>=2.4.2"] |
There was a problem hiding this comment.
Could you expand test_min_dependencies_pyproject_toml to also check minimum version numbers for all of the optional dependency groups?
There was a problem hiding this comment.
I propose to add a test for optional dependencies and think about how to automatically the pyproject.toml in a more convenient way in a separate PR.
Medium-term, if I manage to take some time to look at spin (and convince people it is worth using it), this kind of maintenance tasks could be implemented as spin custom command.
Full disclosure: I did generate the optional dependencies from _min_dependencies.py tag_to_packages
There was a problem hiding this comment.
My last commit d2402e2 added this additional test (and found a few issues in the process e.g. cython 3.0.8 instead of 3.0.9 in pyproject.toml)
This refactored and cleaned up the test to check a pyproject section e.g. build-system.requires against a _min_depencies.py tag e.g. build.
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
|
codecov is red but I think this is good enough. Currently the test only covers |
…nto meson-main-build-backend
|
🎉 |
Opening this PR to get some feed-back about people feelings on making Meson our main build backend. Personnally I have used this daily for my scikit-learn and and I have not noticed any real blocker issues.
I kept a single build with setuptools:
pymin_conda_defaults_openblas. No particular reason why I chose it.It could potentially be an option to do a separate doc PR. Two reasons for this:
--no-use-pep517everywherepyproject.toml(thanks to the--no-use-pep517)Some questions I have about the doc:
--no-use-pep517🎉pip install --verbose --no-build-isolation --editable . --config-settings editable-verbose=true). It is quite long but you have to run it once and thenimport sklearnwill recompile as needed. I find having some feed-back when things recompile very reassuring (meson did recompile things and something is happening this is why my script has not started running yet). This new behaviour probably needs to be explained in the doc ...spin buildwhen you modify a Cython file,spin testto test. So changing all the doc one way to explain the new way with meson editable and then in a month or so revert and explain spin may not be a good use of my time. The big advantage ofspinin my mind is simplicity of the command and the fact that it is discoverable on the command-line. You don't have to look at the doc or your shell history each time you need to find the very long-winded command you need to type.Full-disclosure there are two nice-to-have features from meson-python but I don't consider them a real blocker for daily work:
pytest<8in this PR (note the bug only happens withpytest --pyargswhich is used in the CI but I don't think in day-to-day work?). We also skip one test on Meson because of this right now. Hopefully this will be merged and a 0.16.0 will be released in the not-to-far future.