Bump to v1.5.0

[mcencini]

This PR fix conflicts with recent changes to master. These mainly regards plot (only other change concerns mod_grad_axis, which I updated to match the new internal gradient representation).

Specifically, seq.plot() now returns a "SeqPlot" object, which contains the figure handles. In addition to enable re-usability like #308 did, it improves interactivity (which requires the optional mpcursors package, added as optional dependency). Some examples below:

Standard behavior The standard seq.plot() behaviour is preserved:

Code also supports the overlay of multiple plots, as

fig = seq1.plot(time_range=[0.0, 0.012], plot_now=False)
seq2.plot(time_range=[0.0, 0.012], overlay=fig)

resulting in:

New features In addition, following MATLAB toolbox, this class now supports stacked plots:

seq.plot(time_range=[0.0, 0.012], stacked=True)

When mplcursors is installed, we can also datatips by clicking on the waveforms:

we can also add a moving hairline following the cursor by setting show_guides=True:

seq.plot(time_range=[0.0, 0.012], show_guides=True)

which also works when stacked=True (not shown).

The code is also compatible with the recent SoftDelay extension:

For the moment, I disabled tests. I would like to discuss it before re-enabling (@schuenke): this version replace the optional fig1, fig2 arguments of seq.plot() with "overlay" (a SeqPlot object). In some of the test case, we create the empty figure handles and then fill it in plot(). Do we really want this feature? I figured that the "overlay" argument is more oriented towards overlaying existing plots, rather than filling empty handles. Current SeqPlot has fig1, fig2, ax1 and ax2 as attributes, so user can still access them and customize it if they need to. What do you think?

EDIT: I also added an optional PySide6 dependency to address #268 @btasdelen

[github-actions[bot]]

Coverage Report

File	Stmts	Miss	Cover	Missing
/home/runner/.local/lib/python3.12/site-packages/pypulseq
add_gradients.py	137	60	56%	44, 52, 58, 61, 75–86, 92, 125–128, 135–136, 155, 162, 167–263
add_ramps.py	36	36	0%	1–92
align.py	35	4	89%	41, 45, 69, 73
calc_duration.py	25	1	96%	37
calc_ramp.py	220	216	2%	48–359
calc_rf_bandwidth.py	37	28	24%	45–81, 85–89
check_timing.py	96	29	70%	78, 82, 107, 180, 199, 232, 239, 249–293
compress_shape.py	30	1	97%	28
convert.py	40	8	80%	42, 48, 66, 72–73, 82, 88–89
event_lib.py	96	15	84%	6–9, 48–51, 70–71, 205–210, 247
make_adc.py	98	14	86%	77, 80, 90–94, 97, 146, 149, 153, 159, 163, 202, 204, 206, 214
make_adiabatic_pulse.py	132	39	70%	204–208, 228–232, 240–241, 264, 270, 339–358, 462–471, 509–517
make_arbitrary_grad.py	53	15	72%	71, 74, 77, 80, 96–98, 107, 109, 117–121, 130
make_arbitrary_rf.py	75	63	16%	95–179
make_block_pulse.py	48	3	94%	121–125, 128
make_delay.py	9	1	89%	27
make_digital_output_pulse.py	16	2	88%	39, 47
make_extended_trapezoid.py	56	12	79%	67, 70, 76, 82, 85, 88, 91, 94, 116, 134, 136, 139
make_extended_trapezoid_area.py	93	3	97%	52, 227, 230
make_gauss_pulse.py	73	20	73%	139–143, 146–170, 177, 180
make_label.py	22	4	82%	64, 66, 68, 75
make_sigpy_pulse.py	119	30	75%	12–13, 121, 124, 128, 165–169, 173, 176–177, 180–181, 196, 203, 208, 220, 223, 248–258, 272, 275, 305–315
make_sinc_pulse.py	70	10	86%	102, 108, 136–140, 144, 147–148, 151–152, 174
make_soft_delay.py	25	2	92%	102, 120
make_trapezoid.py	111	7	94%	177, 190, 196, 214, 232, 237, 255
make_trigger.py	16	2	88%	44, 52
opts.py	66	9	86%	78, 83, 102, 142, 166–170
points_to_waveform.py	9	1	89%	27
rotate.py	69	14	80%	15, 55, 66–69, 85–90, 112, 119–120
scale_grad.py	30	1	97%	65
sigpy_pulse_opts.py	26	7	73%	34–41
split_gradient.py	39	31	21%	46–103
split_gradient_at.py	70	27	61%	63–90, 110, 114, 118–120, 154–156
traj_to_grad.py	13	9	31%	26–40
/home/runner/.local/lib/python3.12/site-packages/pypulseq/SAR
SAR_calc.py	113	98	13%	33–40, 55–62, 89–108, 129–132, 168–212, 242–246, 264–306
/home/runner/.local/lib/python3.12/site-packages/pypulseq/Sequence
block.py	469	83	82%	63, 66, 74, 80, 95, 103, 109, 120, 123, 126, 134, 139, 148, 159, 167, 207, 209, 213, 225, 274, 278, 294, 319–345, 382–385, 391, 416–418, 421–429, 436, 466–470, 512, 518, 551, 587–594, 611, 621, 647, 685, 703, 706, 724, 738, 765, 844, 881, 905
calc_grad_spectrum.py	81	76	6%	68–190
calc_pns.py	40	31	22%	45–96
ext_test_report.py	144	12	92%	23, 61, 138, 149–150, 237–243
install.py	75	42	44%	31, 52, 69, 71, 112–131, 148, 181–184, 200–212, 254–278
parula.py	4	2	50%	19–86
read_seq.py	382	125	67%	43–44, 91, 94, 106, 111, 117, 124–125, 129, 138, 146–153, 157–160, 163, 175, 179–181, 224, 229, 237–294, 301–316, 331–401, 406–423, 486, 489, 524, 532, 570, 594, 633, 675–679, 794, 805
sequence.py	696	200	71%	10–13, 105–115, 136–149, 196, 261–264, 311, 338, 355, 403, 431, 458–463, 500, 516, 607, 635–644, 656, 678, 719–722, 776, 814, 825–826, 832, 843, 849, 851, 859, 892–900, 1030, 1120, 1126, 1129, 1132, 1169, 1294–1307, 1337, 1365, 1387–1389, 1410, 1473, 1481, 1548, 1559–1572, 1584–1595, 1641–1642, 1653–1671, 1695, 1725–1733, 1765–1891, 1927, 1941–1951, 1955, 1966
write_seq.py	358	176	51%	45, 69, 72–79, 310–533
/home/runner/.local/lib/python3.12/site-packages/pypulseq/utils
cumsum.py	14	1	93%	17
paper_plot.py	57	53	7%	45–121
safe_pns_prediction.py	126	113	10%	50–87, 102–189, 197–214, 222, 244–250, 279–286, 310–336, 344–383, 396–411, 415
seq_plot.py	378	207	45%	20, 97–114, 130–131, 150, 153–154, 161–164, 167–169, 174–216, 226–233, 240–305, 309–315, 319–327, 349, 351, 354, 359–401, 421, 430, 446–447, 450–453, 485, 500–510, 519–521, 539–541, 543–544, 546–547, 580–592, 607–608, 624–641, 682–686, 689
tracing.py	16	6	62%	33–34, 42, 54–55, 75
/home/runner/.local/lib/python3.12/site-packages/pypulseq/utils/siemens
asc_to_hw.py	58	53	9%	21–28, 48–106
readasc.py	48	45	6%	25–100
TOTAL	5255	2047	61%

Tests	Skipped	Failures	Errors	Time
1366	21 :zzz:	0 :x:	0 :fire:	3m 26s :stopwatch:

[mcencini]

P.S: not sure why it failed pre-commit workflow, it did pass locally:

[schuenke]

Merging mcencini:v1.5.0_dev into imr-framework:v1.5.0_dev and afterwards imr-framework:v1.5.0_dev into imr-framework:master seems complicated. Especially, because imr-framework:v1.5.0_dev is not up to date and to be able to review the changes in this PR, we would need to update and solve the conflics in imr-framework:v1.5.0_dev anyway.

If your branch includes all changes, I suggest we change the target branch of this PR directly to imr-framework:master, no?

[mcencini]

Sure! Sorry, I hope it is better now

[schuenke]

P.S: not sure why it failed pre-commit workflow, it did pass locally:

I think I fixed this already at some point. Might got lost during some manual merging?! GitHub is super slow for me atm, I will have a look at it later.

[mcencini]

P.S: not sure why it failed pre-commit workflow, it did pass locally:

I think I fixed this already at some point. Might got lost during some manual merging?! GitHub is super slow for me atm, I will have a look at it later.

Not sure why, but now correctly raises the errors - which I fixed - also locally. Anyway, I did not want to digress from the scope of PR

[schuenke]

Merging mcencini:v1.5.0_dev into imr-framework:v1.5.0_dev and afterwards imr-framework:v1.5.0_dev into imr-framework:master seems complicated. Especially, because imr-framework:v1.5.0_dev is not up to date and to be able to review the changes in this PR, we would need to update and solve the conflics in imr-framework:v1.5.0_dev anyway.

If your branch includes all changes, I suggest we change the target branch of this PR directly to imr-framework:master, no?

Okay, I didn't realize that most of the code has been reviewed already. Not sure whats the best and easiest way here. Because imr-framework:v1.5.0_dev hasn't been updated in between, it's kind of messy at the moment. And I cannot resolve any merge conflicts because of permission issues on your fork.

Maybe we leave it as it is and try to structure it a bit better the next time. @sairamgeethanath wanted to review the changes anyway, no? So maybe it's not to bad to review some of the code a second time.

[mcencini]

Merging mcencini:v1.5.0_dev into imr-framework:v1.5.0_dev and afterwards imr-framework:v1.5.0_dev into imr-framework:master seems complicated. Especially, because imr-framework:v1.5.0_dev is not up to date and to be able to review the changes in this PR, we would need to update and solve the conflics in imr-framework:v1.5.0_dev anyway. If your branch includes all changes, I suggest we change the target branch of this PR directly to imr-framework:master, no?

Okay, I didn't realize that most of the code has been reviewed already. Not sure whats the best and easiest way here. Because imr-framework:v1.5.0_dev hasn't been updated in between, it's kind of messy at the moment. And I cannot resolve any merge conflicts because of permission issues on your fork.

Maybe we leave it as it is and try to structure it a bit better the next time. @sairamgeethanath wanted to review the changes anyway, no? So maybe it's not to bad to review some of the code a second time.

Apologies, I thought that merging master into mcencini:v1.5.0_dev - which was up-to-date with imr-framework:v1.5.0_dev - would be enough to avoid mess. For the future, should I have updated imr-framework:v1.5.0_dev directly instead (even though I am not sure I have the permission)?

Anyway, it is not bad for sure to have some additional review, in case something slipped in the previous round. Thanks again!

[schuenke]

I don't have the perfect overview over the different branches on the different forks, but always updating the dev branch after master was updated would have been the easiest and cleanest way I guess. This would better separate your modifications from other modifications and thus simplify the review process.

Independent of the branch structure, I realized that for example the changes from #308 are not taken into account in your code. It's not obvious, because the plot function was moved into a separate file, but for example the unification of the subplot naming and handling is completely reverted, same for the plot_type parameter, which appears again in your docstring although it's not a parameter anymore. Would also be great to have the init parameters documented in the __init__ method instead of the class docstring itself. Maybe you can have a look at this again and try to merge the changes from #308 into your code.

I am happy to also review everything afterwards.

And don't get me wrong please: IMO it's great work that you have been doing (!!!), but we should make sure that we include all previous adjustments and try to steadily increase the code and documentation quality.

[mcencini]

I apologize if I am missing something about #308, but I had another look and I am not sure I understand what I am missing - besides the changes to support the new format (phase_ppm, freq_ppm), plot stack and interactivity (data tips and moving hairline), the rest of the code in the private _seq_plot() function seems to follow the recent changes. I also do not see any remaining plot_type (which I previously mistakenly left before opening this PR). Would you kindly point me towards the right direction? Thanks again for you support, I really appreciate it!

Sorry, my mistake. Was checking the code offline and obviously I was on an outdated branch. In the online version, it looks good 👍

I will just review the code changes in this PR directly and post individual comments, so that you and others don't have to wait for me reviewing the 60 files.