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

Replace sct_maths with sct_separate_dmri_separate_dwi_and_b0 in tutorial dMRI preprocessing #3643

Merged
merged 5 commits into from Jan 25, 2022

Conversation

joshuacwnewton
Copy link
Member

@joshuacwnewton joshuacwnewton commented Jan 11, 2022

Checklist

GitHub

PR contents

Description

This PR replaces sct_maths with sct_dmri_separate_dwi_and_b0 in batch_processing.sh and our tutorials. The reason for this is to get a mean image that uses only DWI images and excludes b0 images, which should make for a more reliable mask for cropping around the cord during motion correction.

(In reality, the change shouldn't be very noticeable at all (batch_processing.sh tests still passed), but this is still better instruction to give to those following our tutorials.)

Parallel changes were made in the tutorial dataset (spinalcordtoolbox/sct_tutorial_data#8) as well as the SCT Course slide deck.

Linked issues

Partially addresses #3614.

@joshuacwnewton joshuacwnewton added documentation category: readthedocs, sourceforge, or SCT courses script: batch_processing.sh context: labels Jan 11, 2022
@mguaypaq mguaypaq self-requested a review January 12, 2022 22:02
Copy link
Member

@mguaypaq mguaypaq left a comment

Choose a reason for hiding this comment

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

Apart from one filename mention in an image, this change looks good:

  • a text search for dmri_mean (without _dwi in the middle) under tutorials/diffusion-weighted-mri reveals no other instances to change;
  • I followed the tutorial manually with the changes, and everything still works.

@codecov
Copy link

codecov bot commented Jan 18, 2022

Codecov Report

Merging #3643 (5dd3bd3) into master (cbe2e48) will not change coverage.
The diff coverage is n/a.

Flag Coverage Δ
api-tests 24.18% <ø> (ø)
cli-tests 56.66% <ø> (ø)

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

@joshuacwnewton joshuacwnewton merged commit 26b7fe7 into master Jan 25, 2022
@joshuacwnewton joshuacwnewton deleted the jn/3614-replace-sct_maths-dmri-tutorials branch January 25, 2022 17:00
@joshuacwnewton joshuacwnewton added this to the 5.5 milestone Jan 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation category: readthedocs, sourceforge, or SCT courses script: batch_processing.sh context:
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants