Skip to content

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

Merged
merged 3 commits into from
Feb 15, 2017

Conversation

MarcCote
Copy link
Contributor

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) whenever vox_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.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.007%) to 96.015% when pulling 99b56b5 on MarcCote:bf_support_trk_v1 into 7e08d25 on nipy:master.

@codecov-io
Copy link

codecov-io commented Feb 13, 2017

Codecov Report

Merging #512 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            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
Impacted Files Coverage Δ
nibabel/streamlines/tests/test_trk.py 100% <100%> (ø)
nibabel/streamlines/trk.py 95.25% <100%> (+0.71%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d94c73...fcd7376. Read the comment docs.

@matthew-brett
Copy link
Member

Thanks - have a look at : MarcCote#11 - seem reasonable to you?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.009%) to 96.017% when pulling 9ead3a8 on MarcCote:bf_support_trk_v1 into 5d94c73 on nipy:master.

@MarcCote
Copy link
Contributor Author

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
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure no problem.

@matthew-brett
Copy link
Member

OK - looks good apart from the six import - thanks.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 96.02% when pulling fcd7376 on MarcCote:bf_support_trk_v1 into 5d94c73 on nipy:master.

@matthew-brett matthew-brett merged commit eac4f79 into nipy:master Feb 15, 2017
@matthew-brett
Copy link
Member

Thanks, in it goes.

@MarcCote MarcCote deleted the bf_support_trk_v1 branch February 21, 2017 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants