imr-framework / pypulseq

Pulseq in Python
https://pypulseq.readthedocs.io
GNU Affero General Public License v3.0
117 stars 63 forks source link

Sigpy==0.1.23 incompatible with numpy>1.20 #119

Closed wtclarke closed 10 months ago

wtclarke commented 1 year ago

Hi,

I tried installing this package by cloning and then running pip install ., but when running one of the demo scripts python pypulseq/seq_examples/scripts/write_epi_se_rs.py the following error was returned:

Traceback (most recent call last):
  File "/Users/wclarke/Library/CloudStorage/OneDrive-Nexus365/projects/pps/pypulseq/pypulseq/seq_examples/scripts/write_epi_se_rs.py", line 9, in <module>
    import pypulseq as pp
  File "/Users/wclarke/miniconda3/envs/pps/lib/python3.10/site-packages/pypulseq/__init__.py", line 32, in <module>
    from pypulseq.make_adiabatic_pulse import make_adiabatic_pulse
  File "/Users/wclarke/miniconda3/envs/pps/lib/python3.10/site-packages/pypulseq/make_adiabatic_pulse.py", line 5, in <module>
    from sigpy.mri.rf import hypsec, wurst
  File "/Users/wclarke/miniconda3/envs/pps/lib/python3.10/site-packages/sigpy/__init__.py", line 12, in <module>
    from sigpy import alg, app, config, linop, prox
  File "/Users/wclarke/miniconda3/envs/pps/lib/python3.10/site-packages/sigpy/alg.py", line 8, in <module>
    from sigpy import backend, util
  File "/Users/wclarke/miniconda3/envs/pps/lib/python3.10/site-packages/sigpy/util.py", line 236, in <module>
    def dirac(shape, dtype=np.float, device=backend.cpu_device):
  File "/Users/wclarke/miniconda3/envs/pps/lib/python3.10/site-packages/numpy/__init__.py", line 305, in __getattr__
    raise AttributeError(__former_attrs__[attr])
AttributeError: module 'numpy' has no attribute 'float'.
`np.float` was a deprecated alias for the builtin `float`. To avoid this error in existing code, use `float` by itself. Doing this will not modify any behavior and is safe. If you specifically wanted the numpy scalar type, use `np.float64` here.
The aliases was originally deprecated in NumPy 1.20; for more details and guidance see the original release note at:
    https://numpy.org/devdocs/release/1.20.0-notes.html#deprecations. Did you mean: 'cfloat'?

The sigpy utils.Dirac function has np.float as a default kwarg which is depreciated as of numpy 1.20. This remains the case in the most recent sigpy version https://github.com/mikgroup/sigpy/blob/master/sigpy/util.py#L236.

I think you either need to pin the numpy version to <1.20 or see if the sigpy devs will update their code. Pinning to an old version of numpy will likely also limit the compatible python versions to <= 3.8. I was able to get everything to run by creating an environment using conda create -n pps -c conda-forge python=3.8, modifying the setup.py file to "numpy==1.19.5" and running pip install ..

To Reproduce

Expected behavior The example script should run.

Screenshots N/A

Desktop (please complete the following information):

Additional context I would highly recommend trying out the Github Actions CI to catch these types of issues. I've found it very useful for my open tools.

wtclarke commented 1 year ago

It looks like there is already some progress on this with sigpy https://github.com/mikgroup/sigpy/issues/123 and https://github.com/mikgroup/sigpy/pull/126

btasdelen commented 1 year ago

@wtclarke Thanks for reporting this. We can update the sigpy version as soon as they merge the fix and release it.

I agree with your suggestion. I have never used CI, would you be willing to open a PR for a .yml file for this project? Is that even how it works? :smile:

schuenke commented 1 year ago

Is there a reason why we ask for sigpy==0.1.23 and not >= 0.1.23 ?

We should at least push a fix for the imcompatibility problem to the dev branch, but I could imagine that a lot of users that are not on GitHub get frustrated due to this issue and maybe even stop using PyPulseq...

So imo we should fix this and release v1.40.post1 as soon as possible!!!

btasdelen commented 1 year ago

@schuenke We can do that. Or, we can remove the sigpy dependency, as we are only using a couple of functions from a single file. With appropriate copyright notice of course. How about it?

schuenke commented 1 year ago

I am currently preparing a pypulseq hands-on session for this friday and installing PyPulseq is really a pain in the a**

Can we please push a hotfix to master and release a version 1.40.post1? The only thing we have to do is change the dependencies from "sigpy==0.1.23" to "sigpy>=0.1.25" We can still discuss to remove the sigpy dependency completely in the future, but for now we need a quick solution IMO!

EDIT: the sigpy developers merged the fix in May, but didn't release it yet. Thus, changing to "sigpy>=0.1.25" is NOT enough yet.

A temporary option would be to change the dependency to git+https://github.com/mikgroup/sigpy.git, but this would prohibit a release to PyPi. Still it might be the best option for the moment?

wtclarke commented 10 months ago

This is fixed with the merge of #148