RF: Move sigpy to an extra optional dependency

[wtclarke]

• Vendor our own adiabatic pulse functions in make_adiabatic_pulse.py
• Only enable make_sigpy_pulse.py if sigpy is installed as an extra.
• Add test function for make_adiabatic_pulse.py
• Add markers to disable tests on sigpy pulses if not present
• Update the CI tests. Note that the Run pytest[sigpy] step in push_pr.yml is currently allowed to fail due to #183 .

[wtclarke]

@schuenke All good for review.

[wtclarke]

Thanks for reviewing this in depth. As a general rule, don't refactor additional code in this sort of PR as it just complicates the history. E.g. If we find a bug down the line, is it a optional variable rename or was it me removing the sigpy dep?

And remember PEP8 unto thyself, not unto others (a video well worth watching).

[schuenke]

Thanks for reviewing this in depth. As a general rule, don't refactor additional code in this sort of PR as it just complicates the history. E.g. If we find a bug down the line, is it a optional variable rename or was it me removing the sigpy dep?

And remember PEP8 unto thyself, not unto others (a video well worth watching).

Well, in general I agree, but the current PyPulseq code is basically not tested at all and far away from beeing stable.

Sure, I could have created another PR for the refactoring. And another one for the bug fixe(s). And maybe even a next one for the variable renaming. But in all cases our tests would have passed (although the code was not even working - see the padding code that used invalid numpy function calls) and future debugging wouldn't have been easier imo.

So for the moment, I would still refactor the code in every file that is touched by a PR anyway. To be honest, I think this will also lead to a better code review, because I can almost guarantee that nobody will ever review a PR that refactors all modules and functions. At least not in detail.

[wtclarke]

Fair points. Are you able to merge? Or is it someone else?