-
Notifications
You must be signed in to change notification settings - Fork 262
BF: Supports reading TRK (version 1) #512
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #512 +/- ##
==========================================
+ Coverage 94.03% 94.04% +0.01%
==========================================
Files 166 166
Lines 21995 22017 +22
Branches 2343 2343
==========================================
+ Hits 20682 20706 +24
+ Misses 878 877 -1
+ Partials 435 434 -1
Continue to review full report at Codecov.
|
Thanks - have a look at : MarcCote#11 - seem reasonable to you? |
Also: slightly extend tests for version change.
a40e007
to
9ead3a8
Compare
I rebased, it should be all good. |
import copy | ||
import unittest | ||
import numpy as np | ||
from os.path import join as pjoin | ||
|
||
from six import BytesIO | ||
from nibabel.externals.six import BytesIO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind reverting this to from six import BytesIO
? We're trying to remove use of the nibabel.externals.six
module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure no problem.
OK - looks good apart from the |
Thanks, in it goes. |
Right now, the streamlines API is not able to load TRK (version 1) since it misses the
vox_to_ras
field. This PR fixes that.With this PR, when loading a TRK (version 1), the
vox_to_ras
field is set so it contains all zeros. The rest of the code can already handle this case since in TRK (version 2) whenevervox_to_ras[3][3]
is 0, it means the matrix is not recorded. The way it is curently handled is we assume the affine transformation is the identity matrix and we warn the user.In addition, this PR corrects some typos and changes a bit how we print the TRK header such that only the right amount of scalars and properties names are being displayed.