Skip to content

add scipy to runtime dependency #187

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

Closed
wants to merge 3 commits into from
Closed

add scipy to runtime dependency #187

wants to merge 3 commits into from

Conversation

vtavana
Copy link
Collaborator

@vtavana vtavana commented May 17, 2025

In #179, we added helper functions directly from SciPy for expanding the functionality of mkl_fft.interfaces.scipy_fft module. This addition means that scipy is now a runtime dependency. In this PR it is added to the project as a runtime dependency.

@vtavana vtavana self-assigned this May 17, 2025
@vtavana vtavana marked this pull request as ready for review May 17, 2025 16:44
@Copilot Copilot AI review requested due to automatic review settings May 17, 2025 16:44
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 adds SciPy as a runtime dependency to support the newly imported helper functions from SciPy in the mkl_fft.interfaces.scipy_fft module.

  • Add scipy to the core dependencies in pyproject.toml
  • Remove scipy from the optional test extras
  • Include scipy in both conda recipe metadata files

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
pyproject.toml Added scipy to dependencies and removed from tests
conda-recipe/meta.yaml Added scipy under requirements
conda-recipe-cf/meta.yaml Added scipy under requirements

@vtavana vtavana force-pushed the scipy-runtime branch 5 times, most recently from 645f528 to a9213ed Compare May 18, 2025 16:48
@vtavana
Copy link
Collaborator Author

vtavana commented Jun 3, 2025

@ekomarova needs this PR to be merged to ensure wheel packages function correctly.
And to have a green CI for this PR, we need to first merge #190

@ndgrigorian @antonwolfy When you have time, please review them.

@ndgrigorian
Copy link
Collaborator

My only concern is, does this mean that Intel NumPy can no longer be installed without SciPy, if mkl_fft is being used? Is this something we should do intentionally?

@ekomarova does infra team have concerns about this?

@vtavana
Copy link
Collaborator Author

vtavana commented Jun 3, 2025

My only concern is, does this mean that Intel NumPy can no longer be installed without SciPy, if mkl_fft is being used? Is this something we should do intentionally?

Alternatively, I can modify mkl_fft/interfaces/__init__.py to have

try:
    import scipy.fft
except ImportError:
    pass
else:
    from . import scipy_fft

With this change, Intel NumPy should work fine without a dependency on scipy.
For standalone mkl_fft, there is no need to install scipy as well except for running tests (as before) or if someone wants to use mkl_fft.interface for scipy (new requirement).

@ekomarova
Copy link
Collaborator

ekomarova commented Jun 3, 2025

My only concern is, does this mean that Intel NumPy can no longer be installed without SciPy, if mkl_fft is being used? Is this something we should do intentionally?

@ekomarova does infra team have concerns about this?

Since numpy depends on fft, and fft depends on scipy, then yes. But we only rely on the dependencies of these sources. If this can be changed, and making a dependency on scipy is not necessary for fft, then this will solve the problem with numpy. Then what @vtavana suggests here #187 (comment) would be the alternative solution

@ndgrigorian
Copy link
Collaborator

@vtavana
I think that's sensible, to avoid introducing a strict dependency.

An additional alternative is that tests that use SciPy could be skipped without it in the environment, but that isn't strictly necessary I think.

@vtavana
Copy link
Collaborator Author

vtavana commented Jun 3, 2025

superseded by #195

@vtavana vtavana closed this Jun 3, 2025
@vtavana vtavana deleted the scipy-runtime branch June 3, 2025 20: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.

3 participants