search

imr-framework / pypulseq

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

Remove sigpy dependency and add the files defining RF directly. #134

Closed btasdelen closed 7 months ago

btasdelen commented 9 months ago

This PR aims to remove SigPy dependency.

What works: make_adiabatic_pulse seems to be working.

What is missing: make_sigpy_pulse still requires sigpy. The reason is, it uses plot functions from sigpy, and it requires a much larger part of the library. If those plot lines are replaced or removed, we can completely eliminate the dependency.

schuenke commented 9 months ago

I am not 100% convinced that this is the way to go.

As you describe, make_sigpy_pulse uses a larger part of the sigpy library and if we remove this additional functionality (e.g. the plot methods), we will break the backwards compatibility. Thus, we either have to include all the functionality into pypulseq, as well, or we would need to change the major version of pypulseq if we want to stick with semantic versioning.

Therefore, I would still suggest that we temporarily change the dependency in our dev branch to git+https://github.com/mikgroup/sigpy.git (we cannot release this to PyPi !) and change it back to sigpy>0.1.25 as soon as the sigpy developers release their fix (will probably be version 0.1.26).

btasdelen commented 7 months ago

As discussed during the call, problems related to SigPy are solvable, so this PR is unnecessary for now.