Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENH: Add support for ASL data #411

Merged
merged 5 commits into from
Dec 5, 2023
Merged

ENH: Add support for ASL data #411

merged 5 commits into from
Dec 5, 2023

Conversation

tsalo
Copy link
Contributor

@tsalo tsalo commented Dec 1, 2023

Closes none, but is related to #405. I'm happy to take a crack at adding tests, but it would be good to know what the preferred approach is. The easiest thing might be for me to just copy existing test files and rename them. I don't think distortion correction of EPI ASL will be any different from SDC of other EPI sequences, but I could add actual ASL and M0 data if that would be better. Unfortunately, I don't have access to data from a number of the cases listed below.

There are a few cases that I think should be covered. Do they make sense?

  1. ASL scan with AP and PA M0 scans located in fmap, with separate M0 scan in perf.
  2. AP ASL scan with PA M0 scan in fmap, with separate M0 scan in perf.
  3. AP and PA ASL scans (is that supported yet?), with separate M0 scan in perf.
  4. ASL scan with phase-difference field maps, with separate M0 scan in perf.
  5. ASL scan without any field map.

Changes proposed:

  • Add asl and m0scan to PEPOLAR and fieldmap-less suffix lists.
    • I essentially added "asl" wherever I saw "dwi".

Copy link

codecov bot commented Dec 1, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (53482c1) 80.59% compared to head (3ef588a) 80.59%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #411   +/-   ##
=======================================
  Coverage   80.59%   80.59%           
=======================================
  Files          26       26           
  Lines        2293     2293           
  Branches      291      291           
=======================================
  Hits         1848     1848           
  Misses        401      401           
  Partials       44       44           
Flag Coverage Δ
unittests ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

oesteban
oesteban previously approved these changes Dec 4, 2023
Copy link
Member

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

LGTM - just added a minor comment about m0scans

sdcflows/utils/wrangler.py Show resolved Hide resolved
Co-authored-by: Oscar Esteban <code@oscaresteban.es>
@oesteban oesteban merged commit ee5b805 into nipreps:master Dec 5, 2023
21 checks passed
@tsalo tsalo deleted the add-asl branch December 5, 2023 16:01
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.

2 participants