Skip to content

Make mkl-service an optional dependency#305

Open
ndgrigorian wants to merge 1 commit intomasterfrom
drop-mkl-service-dependency
Open

Make mkl-service an optional dependency#305
ndgrigorian wants to merge 1 commit intomasterfrom
drop-mkl-service-dependency

Conversation

@ndgrigorian
Copy link
Copy Markdown
Collaborator

mkl-service is only a dependency for the sake of the scipy interface, which is optional

This PR proposes moving mkl-service to optionally installable and adding logic to check for both mkl-service and scipy in the environment before exposing the scipy interface

Copilot AI review requested due to automatic review settings April 9, 2026 21:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-service out of core dependencies and into optional extras in pyproject.toml.
  • Updates mkl_fft.interfaces to only import/export scipy_fft when both SciPy and mkl-service are available.
  • Adjusts conda recipes and CI workflows to install mkl-service explicitly 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).

Comment on lines +33 to +40
_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
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@ndgrigorian ndgrigorian force-pushed the drop-mkl-service-dependency branch 2 times, most recently from aaf2822 to 81767b3 Compare April 9, 2026 21:36
mkl-service is only a dependency for the sake of the scipy interface, which is optional
@ndgrigorian ndgrigorian force-pushed the drop-mkl-service-dependency branch from 81767b3 to a8169a5 Compare April 9, 2026 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants