Skip to content

Improve input validation in all make_* functions - #433

Merged
schuenke merged 9 commits into
masterfrom
input_validation
Oct 29, 2026
Merged

schuenke merged 9 commits into
masterfrom
input_validation

Conversation

@schuenke

@schuenke schuenke commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Motivated by #398 and future refactoring plans, I added input validations for almost all arguments of our make_* functions.
While touching the files, I also standardised the docstrings, added missing parameters etc.

This leads to many code changes, however, 99.9% do NOT affect the code itself. The only (slight) logic change is in make_block_pulse, because I didn't like the if elif cases with a pass statement in the middle of the block.

And I added the duration attribute to ADC when giving it as an argument. This was missing before.

I also added tests for the new input validations.

Closes #398

@schuenke
schuenke requested review from ckolbPTB and mcencini October 20, 2026 06:36
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Coverage

Coverage Report
FileStmtsMissCoverMissing
/home/runner/.local/lib/python3.12/site-packages/pypulseq
   add_gradients.py1372185%44, 52, 61, 85, 92, 125–128, 155, 162, 177, 187–190, 211–218, 225, 230, 261
   add_ramps.py36360%1–92
   align.py35489%41, 45, 69, 73
   calc_duration.py33294%43, 50
   calc_ramp.py2202162%48–359
   calc_rf_bandwidth.py372824%45–81, 85–89
   check_timing.py992872%78, 82, 184, 203, 236, 243, 253–297
   compress_shape.py30197%28
   convert.py40880%42, 48, 66, 72–73, 82, 88–89
   event_lib.py961584%6–9, 48–51, 70–71, 185, 205–210
   make_adc.py1081091%114, 163, 166, 170, 176, 180, 219, 221, 223, 231
   make_adiabatic_pulse.py1503974%214–218, 238–242, 250–251, 274, 280, 349–368, 472–481, 519–527
   make_arbitrary_grad.py62297%124, 145
   make_arbitrary_rf.py832471%145–149, 152–156, 166–185, 188, 191
   make_block_pulse.py53198%132
   make_delay.py9189%27
   make_extended_trapezoid.py64691%74, 77, 137, 154, 156, 159
   make_extended_trapezoid_area.py110496%71, 74, 273, 276
   make_gauss_pulse.py90594%193, 201–203, 218
   make_sigpy_pulse.py1292878%12–13, 134, 137, 186–190, 197–198, 201–202, 217, 224, 229, 267, 270, 295–305, 345, 348, 378–388
   make_sinc_pulse.py84495%196–198, 213
   make_soft_delay.py32197%136
   make_trapezoid.py128795%207, 221, 227, 245, 263, 268, 286
   opts.py66986%78, 83, 102, 142, 166–170
   points_to_waveform.py9189%27
   rotate.py69593%55, 69, 112, 119–120
   rotate_3d.py78297%145–146
   sigpy_pulse_opts.py28775%66–73
   split_gradient.py393121%46–103
   split_gradient_at.py702761%63–90, 110, 114, 118–120, 154–156
   traj_to_grad.py13931%26–40
/home/runner/.local/lib/python3.12/site-packages/pypulseq/SAR
   SAR_calc.py5180%9
/home/runner/.local/lib/python3.12/site-packages/pypulseq/Sequence
   block.py4525488%64, 67, 75, 81, 96, 104, 110, 121, 124, 127, 135, 140, 149, 160, 168, 208, 210, 214, 329, 333, 336, 357, 372, 382, 393, 402, 434, 438, 447, 460, 471, 480–484, 502, 534–541, 558, 568, 594, 632, 671, 685, 712, 791, 828, 852
   calc_grad_spectrum.py81766%68–190
   calc_pns.py403122%45–96
   ext_grad_check.py55296%51, 53
   ext_test_report.py1671193%79, 156, 167–168, 337–343
   install.py754244%31, 52, 69, 71, 112–131, 148, 181–184, 200–212, 254–278
   parula.py4250%19–86
   read_seq.py4084190%44–45, 110, 117, 130, 133–134, 138, 184, 223–224, 242, 415, 436–453, 516, 519, 604, 628, 668, 699, 715–719, 726, 837, 848
   sequence.py64113379%10–13, 107–117, 138–151, 205, 270–273, 320, 364, 440, 467–472, 509, 537–540, 631, 659–668, 680, 702, 743–746, 775, 801–807, 842, 853–854, 860, 871, 877, 879, 887, 920–928, 1060, 1158, 1164, 1167, 1170, 1209, 1334–1347, 1405, 1427–1429, 1450, 1513, 1521, 1619, 1630–1643, 1712–1713, 1724–1742, 1766, 1796–1804, 1840, 1854–1864
   write_seq.py362798%45, 77–79, 313, 347–349, 499
/home/runner/.local/lib/python3.12/site-packages/pypulseq/utils
   cumsum.py14193%17
   paper_plot.py63588%49–132
   safe_pns_prediction.py12611310%50–87, 102–189, 197–214, 222, 244–250, 279–286, 310–336, 344–383, 396–411, 415
   seq_plot.py40718654%22, 106, 138–139, 159, 176–178, 183–225, 235–242, 249–314

Warning: Comment truncated due to GitHub's 65536 character limit

@schuenke

Copy link
Copy Markdown
Collaborator Author

Short explanation for the last commit (13d9c59):

  • In the first commit, I already changed make_adc to always set adc.duration. Previously, adc.duration was only set in the dwell-path, which was inconsistent IMO.

  • To keep event objects consistent regardless of how they're created, Sequence.get_block() reconstruction now restores adc.duration as num_samples * dwell, the same formula make_adc and calc_duration use. Therefore, users can rely on the attribute existing on every ADC event now.

  • Making sure adc.duration always exists pointed out a latent bug: check_timing div-checked adc.duration against rf_raster_time, which produced many false-positive RASTER errors in epi_se_rs and mprage. The reason why it was checking against rf_raster_time is that adc.delay has to lie on the RF raster, which we are/were correctly checking before in the code. The (wrong) duration check was previously never triggered, because reconstructed ADCs (using seq.get_block() in check_timing()) never had a duration attribute.

  • check_timing() now correctly div-checks adc.delay against rf_raster_time and adc.duration against adc_raster_time for all ADC events independent of how they were created and/or restored.

@mcencini mcencini left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great job!

@schuenke
schuenke merged commit a772da0 into master Oct 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing conditional check for Duration

2 participants