Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes mkl-service optional by moving it into the scipy_interface/test extras and conditionally exposing the SciPy adapter only when both SciPy and the mkl Python module (from mkl-service) are present.
Changes:
- Moves
mkl-serviceout of core dependencies and into optional extras inpyproject.toml. - Updates
mkl_fft.interfacesto only import/exportscipy_fftwhen both SciPy andmkl-serviceare available. - Adjusts conda recipes and CI workflows to install
mkl-serviceexplicitly for SciPy-related testing.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Shifts mkl-service into extras and changes base dependency set. |
mkl_fft/interfaces/__init__.py |
Adds runtime checks to gate the SciPy interface export. |
conda-recipe/meta.yaml |
Moves runtime MKL dependency to mkl and adds mkl-service to test requirements. |
conda-recipe-cf/meta.yaml |
Same dependency adjustments for conda-forge recipe. |
.github/workflows/conda-package.yml |
Ensures mkl-service is installed for test envs that include SciPy. |
.github/workflows/conda-package-cf.yml |
Same test-environment adjustments for conda-forge workflow. |
.github/workflows/build_pip.yaml |
Updates MKL install and adds explicit mkl-service install for tests (currently redundant with extras). |
| _has_scipy = importlib.util.find_spec("scipy") is not None | ||
| _has_mkl_service = importlib.util.find_spec("mkl") is not None | ||
|
|
||
| if _has_scipy: | ||
| if not _has_mkl_service: | ||
| pass | ||
| else: | ||
| from . import scipy_fft |
There was a problem hiding this comment.
find_spec("scipy") only checks that a module is discoverable, not that it can actually be imported. If SciPy is present but import fails at runtime (e.g., missing shared libs / ABI issues), this will still attempt to import scipy_fft and can make import mkl_fft.interfaces fail. Consider reverting to a try: import scipy (and similarly import mkl) guard like the previous implementation so failures are safely ignored.
aaf2822 to
81767b3
Compare
mkl-service is only a dependency for the sake of the scipy interface, which is optional
81767b3 to
a8169a5
Compare
mkl-serviceis only a dependency for the sake of thescipyinterface, which is optionalThis PR proposes moving
mkl-serviceto optionally installable and adding logic to check for bothmkl-serviceandscipyin the environment before exposing thescipyinterface