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

dwi2response: Fix for absent -mask and no gradient table #1346

Merged
merged 1 commit into from May 21, 2018

Conversation

Projects
None yet
2 participants
@Lestropie
Copy link
Member

Lestropie commented May 21, 2018

Came across this one myself. Details in commit message of 19eaa28.

dwi2response: Fix for absent -mask and no gradient table
If a mask is not provided to the script via the -mask option, the script will automatically generate a brain mask using the dwi2mask command. However, this command was being invoked upon the input image as provided by the user. If this image did not contain a gradient table in the header, and the user was relying on the -grad or -fslgrad option to provide this information, the dwi2mask call would fail. This change ensures that the dwi2mask command is executed on the DWI that has been imported into the temporary directory, and therefore is guaranteed to contain a diffusion gradient table.

@Lestropie Lestropie self-assigned this May 21, 2018

@Lestropie Lestropie requested a review from thijsdhollander May 21, 2018

@thijsdhollander
Copy link
Member

thijsdhollander left a comment

Nice catch 👍 . I haven't tested the solution, but looked over the diff in detail. It looks all good; I can't imagine a scenario that would fail. Running the mask estimation on dwi.mif in the script temp folder should indeed also be safer, and can guarantee only a properly checked/processed imported dwi.mif gets fed to dwi2mask. Happy to merge ASAP!

@Lestropie

This comment has been minimized.

Copy link
Member

Lestropie commented May 21, 2018

I tested different combinations of -mask / -grad / fslgrad & lack thereof, so should be good.

@Lestropie Lestropie merged commit 8cef832 into master May 21, 2018

1 check passed

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

@Lestropie Lestropie deleted the dwi2response_hotfix branch May 21, 2018

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