imr-framework / pypulseq

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

Dev-Branch: Importing from the package during setup ignores dependencies #73

Closed fzimmermann89 closed 2 years ago

fzimmermann89 commented 2 years ago

Hi,

A pip install of the current dev branch fails in a new environment:

  File "/tmp/pip-install-lm9p2ev9/pypulseq_35f87ebee4c24524a362107451789dc0/setup.py", line 3, in <module>
          from pypulseq import major, minor, revision
        File "/tmp/pip-install-lm9p2ev9/pypulseq_35f87ebee4c24524a362107451789dc0/pypulseq/__init__.py", line 60, in <module>
          from pypulseq.make_adiabatic_pulse import make_adiabatic_pulse
        File "/tmp/pip-install-lm9p2ev9/pypulseq_35f87ebee4c24524a362107451789dc0/pypulseq/make_adiabatic_pulse.py", line 5, in <module>
          from sigpy.mri.rf import hypsec, wurst
      ModuleNotFoundError: No module named 'sigpy'
      [end of output]

not sure if this is a wontfix (as its just the dev branch), but importing the version in https://github.com/imr-framework/pypulseq/blob/154e838f8fb47ab33582c669ed843547ee0c5cf6/setup.py#L3 from the package to install causes its __init__ to run, which down the line causes imports of matplotlib, sigpy, scipy etc. This fails in a new environment as these imports are done before the dependencies are installed.

Possible solutions could be to use a pyproject.toml to specify those build requirements or to move the version finding logic somewhere else.

Best Felix

sravan953 commented 2 years ago

Been a while since I tested out clean installs to check if everything works as expected. Thanks for identifying this @fzimmermann89 ! I have come across pyproject.toml files, but it is low-priority for now (PRs are always welcome! 😉).

Also, do you have any ideas if the whole process of creating a venv and pip-installing a package can be unit-tested?

fzimmermann89 commented 2 years ago

Thanks for the quick fix!

It seems you don't "need" a pyproject.toml, so I wouldn't bother.. Unittesting the setup would be difficult, but a github actions pipeline that installs the package in a new environment and runs all unittests could maybe be useful and capture these kind of problems.

Not sure what your opinions about that are, if you are interested I might find the time at some point to prepare some example actions as a PR (but would only spent the time if you would consider using this github feature.. )

Best