Skip to content

conditional import scipy_fft #195

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

Merged
merged 2 commits into from
Jun 3, 2025
Merged

conditional import scipy_fft #195

merged 2 commits into from
Jun 3, 2025

Conversation

vtavana
Copy link
Collaborator

@vtavana vtavana commented Jun 3, 2025

In this PR, mkl_fft.interface is modified to only import scipy_fft if scipy is installed.

@vtavana vtavana self-assigned this Jun 3, 2025
@vtavana vtavana marked this pull request as ready for review June 3, 2025 19:59
@Copilot Copilot AI review requested due to automatic review settings June 3, 2025 19:59
Copy link
Contributor

@Copilot 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 the SciPy FFT interface optional by only importing scipy_fft when SciPy is installed, and adapts packaging, tests, and documentation accordingly.

  • Introduce a new optional dependency group for SciPy in pyproject.toml and update test requirements.
  • Conditionally import scipy_fft in the interface module.
  • Modify tests to skip SciPy‐related tests when SciPy is absent and unify version checks.
  • Update conda recipes and README to reflect the optional SciPy dependency.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pyproject.toml Added scipy_interface extras, tightened test group to SciPy ≥1.10
mkl_fft/interfaces/init.py Wrapped SciPy FFT import in a try/except
mkl_fft/tests/third_party/scipy/*.py Added module-level skips, replaced hard imports and version checks
mkl_fft/tests/test_interfaces.py Dynamic interface list and pytest.importorskip for SciPy tests
conda-recipe/meta.yaml Updated SciPy test requirement to ≥1.10
conda-recipe-cf/meta.yaml Updated SciPy test requirement to ≥1.10
README.md Added optional SciPy install steps
Comments suppressed due to low confidence (3)

conda-recipe/meta.yaml:36

  • Conda tests require SciPy >=1.10, but test_basic.py skips <1.12; update this to scipy >=1.12 for consistency.
-      - scipy >=1.10

conda-recipe-cf/meta.yaml:35

  • Conda-CF tests require SciPy >=1.10, but other tests skip <1.12; consider bumping to scipy >=1.12.
-      - scipy >=1.10

README.md:96

  • [nitpick] The README suggests installing SciPy without a version; consider specifying pip install scipy>=1.12 to match test requirements.
-  - `pip install scipy` (optional: for using `mkl_fft.interface.scipy_fft` module)

Copy link
Collaborator

@ndgrigorian ndgrigorian left a comment

Choose a reason for hiding this comment

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

LGTM!

@vtavana vtavana merged commit 933b8ed into master Jun 3, 2025
57 checks passed
@vtavana vtavana deleted the install-scipy branch June 3, 2025 21:21
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