Conversation
1f7006a to
e209e46
Compare
e209e46 to
dfc8f4c
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates mkl-service from a setuptools/setup.py-driven build to a meson-python build, updating conda recipes and CI workflows accordingly, and adding an mkl runtime dependency.
Changes:
- Remove
setup.pyand define extension builds/install layout inmeson.build(C + Cython extensions, Python sources, tests). - Switch
pyproject.tomlbuild backend tomesonpyand addmklto runtime dependencies. - Update conda recipes and GitHub Actions workflows to build/install via
pip/python -m buildinstead ofsetup.py.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Removed legacy setuptools build script. |
| pyproject.toml | Switched build backend to mesonpy, updated build requirements and runtime deps. |
| meson.build | Added Meson build definition for C/Cython extensions and installation layout. |
| mkl/_py_mkl_service.pyx | Added/introduced Cython wrapper implementation used by the public API. |
| conda-recipe/meta.yaml | Updated conda build requirements for Meson-based builds. |
| conda-recipe/build.sh | Updated wheel build/install flow (no setup.py). |
| conda-recipe/bld.bat | Removed setup.py clean step and MKLROOT usage. |
| conda-recipe-cf/meta.yaml | Updated conda-forge recipe host requirements for Meson-based builds. |
| conda-recipe-cf/build.sh | Switched to pip install . build/install flow. |
| conda-recipe-cf/bld.bat | Switched to pip install . build/install flow. |
| .github/workflows/build-with-standard-clang.yml | New CI job building with system clang. |
| .github/workflows/build-with-clang.yml | Updated IntelLLVM clang CI job to Meson-based install. |
| .github/workflows/build_pip.yml | New CI job testing editable install + (pre-)release NumPy in conda env. |
| 'mkl-service', | ||
| ['c', 'cython'], | ||
| version: run_command( | ||
| 'python', '-c', |
There was a problem hiding this comment.
run_command('python', ...) hardcodes the interpreter, which may not match Meson’s discovered Python (virtualenv/pyenv/Windows). Use the py installation found by import('python').find_installation() when computing the version to avoid mismatched interpreters.
| 'python', '-c', | |
| import('python').find_installation(pure: false), '-c', |
|
|
||
| thread_dep = dependency('threads') | ||
|
|
||
| cc = meson.get_compiler('c') |
There was a problem hiding this comment.
cc = meson.get_compiler('c') is unused. Removing it avoids dead code and potential lint/warning noise during Meson configure.
| cc = meson.get_compiler('c') |
There was a problem hiding this comment.
mkl_random broke without this. Leaving it in. Might document it.
| strategy: | ||
| matrix: | ||
| python: ["3.10", "3.11", "3.12", "3.13", "3.14"] | ||
| numpy_version: ["numpy'>=2'"] |
There was a problem hiding this comment.
The matrix value numpy_version: ["numpy'>=2'"] includes embedded quotes and will be passed verbatim to pip install, which is not a valid requirement specifier. Use something like numpy>=2 (or just install numpy and control pre-releases separately).
| numpy_version: ["numpy'>=2'"] | |
| numpy_version: ["numpy>=2"] |
| strategy: | ||
| matrix: | ||
| python: ["3.10", "3.11", "3.12", "3.13"] | ||
| numpy_version: ["numpy'>=2'"] |
There was a problem hiding this comment.
The matrix value numpy_version: ["numpy'>=2'"] includes embedded quotes and will be passed verbatim to pip install, which is not a valid requirement specifier. Use something like numpy>=2.
| numpy_version: ["numpy'>=2'"] | |
| numpy_version: ["numpy>=2"] |
4a2979d to
b3cd7c9
Compare
b3cd7c9 to
3fb7aff
Compare
This PR proposes moving from
setuptoolstomeson-pythonas themkl-servicebuild systemmeson-pythonis already used by NumPy and allowssetup.pyto be removed (with its logic moved into themeson.buildscript)Also adds mkl as a dependency