Skip to content
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

Fix incompatibility with scipy 1.14.0 and numpy 2.0 #143

Merged
merged 4 commits into from
Nov 2, 2024

Conversation

schuenke
Copy link
Contributor

@schuenke schuenke commented Jun 25, 2024

As described in #142, several changes in the scipy and numpy packages lead to errors in sigpy.

I tried to fix them so that all tests pass now, but I am not a sigpy user myself and can't guarantee that all changes in the function calls, especially the introduction of the kwarg "weight", are actually correct. Would be great if someone could confirm that the one character variable w is supposed to be a weight (although from the scipy changelog I am quite sure that this is the case 😁 )

The rest should be fine.

closes #142

@jweine
Copy link

jweine commented Jul 23, 2024

Maybe it is worth noting that these changes introduce the minimum requirements of:

There for in project.toml specifying should be foolproof. However, as these versions are pretty outdated I am not sure if this is absolutely necessary.

@schuenke
Copy link
Contributor Author

Maybe it is worth noting that these changes introduce the minimum requirements of:

* Scipy 1.6.0 (Renaming with backwards compatibility: [MAINT: rename trapz and cumtrapz to (cumulative_)trapezoid scipy/scipy#12934](https://github.com/scipy/scipy/pull/12934)), the window namespace seems to preceed 1.6.0 anyway)

* Numpy 1.13 (oldest available reference version including `np.inf`: https://numpy.org/doc/1.13/reference/generated/numpy.isinf.html#numpy.isinf)

There for in project.toml specifying should be foolproof. However, as these versions are pretty outdated I am not sure if this is absolutely necessary.

Thanks for checking. Currently, no version numbers are defined in the pyproject.toml at all. Not sure if this is on purpose (if you start it, you need to keep it up to date) or not ?! Maybe one of the maintainers can let me know if I should change it and include it in this PR.

@tito21
Copy link

tito21 commented Aug 13, 2024

Is anyone still looking at this? I think that is important that sigpy works in a modern python ecosystem.

For my use case this branch is working (I only really use the EspiritCalib)

@frankong
Copy link
Member

frankong commented Aug 13, 2024 via email

@gattia
Copy link

gattia commented Oct 9, 2024

Just checking this - I went to make a new PR because of the numpy incompatibility, but then saw that its already fixed here but not merged.

@lrlunin
Copy link

lrlunin commented Oct 29, 2024

Please also bump the numpy version to at least >=1.23.5 in conda-forge distribution https://github.com/conda-forge/sigpy-feedstock .

Copy link
Member

@jtamir jtamir left a comment

Choose a reason for hiding this comment

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

Straightforward changes, thank you!

@jtamir jtamir merged commit be389d2 into mikgroup:main Nov 2, 2024
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.

Sigpy incompatible with scipy 1.14.0 and numpy 2.0
7 participants