Fix inconsistency in "return_delay" argument of RF pulses

[mavel101]

This PR fixes an inconsistent behaviour of the "return_delay" argument for RF pulses. Currently, delays are returned only, if also "return_gz" is set to True, except for adiabatic pulses. However, sometimes it is useful to return a delay, even when no slice gradient is desired. The behaviour is now the same as in make_adiabatic_pulse.py

This PR also replaces the deprecated zero-filling at the end of an RF pulse for sigpy pulses by returning a delay instead.

[FrankZijlstra]

I have a few queries regarding this delay event, most of which isn't so much related to this PR, but worth having a little discussion about:

• What is the purpose of the delay event in the first place? The ringdown time is already present in calc_duration, so any RF event will already have at least the duration of the delay. It seems very niche to need a delay event in case you want to replace an RF event with a delay instead, and easily achieved with make_delay(calc_duration(rf)). I don't see the purpose of calculating this delay event in the first place.
• In calculating the delay, the ringdown time is added to the RF duration as returned by calc_duration, which already includes the ringdown time. This seems wrong.
• The zero-padding of the sigpy pulses seems weird to have been there in the first place. Again, the ringdown time is already reserved through calc_duration. Removing that code seems correct, but as per the first query, I don't think any delay event is necessary?

If there is a purpose for the delay event, I see no problems with the PR. Otherwise, I suggest removing it altogether.

@schuenke @btasdelen What are your thoughts?

[btasdelen]

• I would assume the purpose is to have the delay if one wants to add a slice rewinder in the same block? But then, the calculation would be make_delay(calc_duration(rf) - rf.ringdown_time), not (+)? In any case, this is a one-liner, and calculating such timings are use case dependent and better done outside, IMHO.
• I agree, maybe it is an artifact of old code when calc_duration() was not taking it into account? Matlab Pulseq has the same issue, btw.

  delay=mr.makeDelay(mr.calcDuration(rf)+rf.ringdownTime);

• Similarly, I think that is a left-over from pre v1.4 times.

I am for removing it.

[mavel101]

I agree, the ringdown time is already covered in calc_duration. A very specific use case would be to play an RF pulse, which is not aligned to the gradient raster time. However, this is not covered by the current implementation and could easily be done outside.

If you wish, I could remove all return_delay arguments and outputs and update this PR.

[FrankZijlstra]

Yes, it would be good if you could remove the delay outputs. Do note that I merged another PR recently, so it's probably easiest to just do a hard reset on your branch.

[mavel101]

I removed the delay outputs and also removed one test, that checked the delay object of the adiabatic pulse.

[FrankZijlstra]

Thanks for the fix, looks good to me. One small thing: Could you remove the return type hints that included the delay? E.g. in make_sinc_pulse that would be Tuple[SimpleNamespace, SimpleNamespace, SimpleNamespace, SimpleNamespace],

[mavel101]

I removed them along with some unnecessary imports

[btasdelen]

I think this PR is ready, any other comments @FrankZijlstra?

[FrankZijlstra]

No comments, just forgot 😂. Will merge it now.