3Tissue / MRtrix3Tissue

MRtrix3Tissue adds capabilities for 3-Tissue CSD modelling and analysis to a complete version of MRtrix3.
https://3Tissue.github.io
Mozilla Public License 2.0
10 stars 6 forks source link

read_mrtrix.m fails to read output from ss3t_csd_beta1 #16

Closed schoffelen closed 4 years ago

schoffelen commented 4 years ago

Thanks for the awesome software. I have just recently started working on tractography data and I am really impressed by the software, its documentation, and its open community spirit. I am working on some older single shell data, and therefore was interested in trying out the MRTrix3Tissue. In trying to load in a generated wmfod.mif file, created by ss3t_csd_beta1, into MATLAB, readmrtrix.m fails, because it tries to assign a fieldname with a '-', which is not allowed. I fixed this locally, by checking for a '-', and replacing it with ''. I'd be happy to submit this fix as a PR.

thijsdhollander commented 4 years ago

Thanks for the awesome software.

Thanks! 👍

I have just recently started working on tractography data and I am really impressed by the software, its documentation, and its open community spirit. I am working on some older single shell data, and therefore was interested in trying out the MRTrix3Tissue.

No worries at all; glad to hear! I hope you're happy with the results so far? Always feel free to post some feedback of course; or even if you're just wondering whether the results that pop out look sensible or have decent quality.

In trying to load in a generated wmfod.mif file, created by ss3t_csd_beta1, into MATLAB, read_mrtrix.m fails, because it tries to assign a fieldname with a '-', which is not allowed.

Interesting, well spotted. I might in the future consider even not using the dash in those header entries for that reason. But in any case, the read_mrtrix.m should probably not stumble over it either.

I fixed this locally, by checking for a '-', and replacing it with '_'. I'd be happy to submit this fix as a PR.

It'd be the first time to get an external PR in a fork; I'm guessing this should work though! Feel free to give it a go, and I guess we'll see what happens. 😄

thijsdhollander commented 4 years ago

Ok, looks like we've got this fixed up now with #18. 👍 I'll close this issue; feel free to provide some feedback anytime!