-
Notifications
You must be signed in to change notification settings - Fork 21
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
Conversation
There was a problem hiding this 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 optionaltest
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 |
645f528
to
a9213ed
Compare
@ekomarova needs this PR to be merged to ensure wheel packages function correctly. @ndgrigorian @antonwolfy When you have time, please review them. |
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? |
Alternatively, I can modify try:
import scipy.fft
except ImportError:
pass
else:
from . import scipy_fft With this change, Intel NumPy should work fine without a dependency on |
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 |
@vtavana 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. |
superseded by #195 |
In #179, we added helper functions directly from SciPy for expanding the functionality of
mkl_fft.interfaces.scipy_fft
module. This addition means thatscipy
is now a runtime dependency. In this PR it is added to the project as a runtime dependency.