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

DICOM: Fix slice timing import for mosaic data #1359

Merged
merged 1 commit into from Jun 2, 2018

Conversation

Projects
None yet
3 participants
@Lestropie
Copy link
Member

Lestropie commented Jun 1, 2018

The wrong member function of class File::DICOM::CSAEntry was being used to pre-allocate the memory required to store slice timing, resulting in a mismatch in length between this data vector and the number of slices in the image.

For the sake of web searches, the issue that this fix corrects looks something like:

[WARNING] Number of entries in mosaic slice timing (82) does not match number of images in mosaic (60); omitting

Closes #1252.

Should probably wait for @jdtournier to run against private DICOM test suite.

DICOM: Fix slice timing import for mosaic data
The wrong member function of class File::DICOM::CSAEntry was being used to pre-allocate the memory required to store slice timing, resulting in a mismatch in length between this data vector and the number of slices in the image.
Closes #1252.

@Lestropie Lestropie added the bug label Jun 1, 2018

@Lestropie Lestropie self-assigned this Jun 1, 2018

@Lestropie Lestropie requested a review from jdtournier Jun 1, 2018

@Lestropie

This comment has been minimized.

Copy link
Member

Lestropie commented Jun 1, 2018

Note: This only affects the ability of MRtrix3 to create the key-value entry "SliceTiming" in the image header, which may then be used by dwipreproc when instructing eddy to use slice-to-volume motion correction. Otherwise, it's just an annoying erroneous warning.

@thijsdhollander

This comment has been minimized.

Copy link
Member

thijsdhollander commented Jun 1, 2018

Thanks for looking into it! Should definitely go to master once tested by Donald. It's making a lot of people panic / worry.

@jdtournier

This comment has been minimized.

Copy link
Member

jdtournier commented Jun 2, 2018

OK, just put this through its paces on my DICOM test suite, it's all good!

@jdtournier jdtournier merged commit 92e06e0 into master Jun 2, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jdtournier jdtournier deleted the dicom_mosaic_slice_timing_fix branch Jun 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment