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

Removed affine matrices from tracking. #1560

Merged
merged 2 commits into from Aug 3, 2018

Conversation

Projects
None yet
5 participants
@albayenes
Copy link
Contributor

albayenes commented Jun 10, 2018

Because Trackvis uses LPS but DIPY uses RAS, using affine with both tracking and saving as .trk file causes incorrect display in Trackvis. So, affine matrices removed from tracking functions to perform tracking in voxel coordinates.

@pep8speaks

This comment has been minimized.

Copy link

pep8speaks commented Jun 10, 2018

Hello @albayenes, Thank you for updating !

Line 87:1: E402 module level import not at top of file

Comment last updated on August 03, 2018 at 11:47 Hours UTC
@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jun 12, 2018

Looks great!

screen shot 2018-06-12 at 2 52 13 pm

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jun 12, 2018

For reference, this is what it looks like on current master
screen shot 2018-06-12 at 2 55 18 pm

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jun 12, 2018

+1 for the merge from me! Thanks for fixing this up @albayenes !

@skoudoro

This comment has been minimized.

Copy link
Member

skoudoro commented Jul 31, 2018

+1 for merging this too! Can you rebase this PR @albayenes? It will be ready to go after that!

albayenes added some commits Jun 10, 2018

Removed affine matrices from tracking. Because Trackvis uses LPS but …
…DIPY uses RAS, using affine with both tracking and saving as .trk file causes incorrect display in Trackvis.

@albayenes albayenes force-pushed the albayenes:basic-traking-removed-tracking-affine-to-visualize-trakvis-correct branch from e97668e to c6bbada Aug 3, 2018

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Aug 3, 2018

Codecov Report

Merging #1560 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1560   +/-   ##
=======================================
  Coverage   87.34%   87.34%           
=======================================
  Files         246      246           
  Lines       31811    31811           
  Branches     3451     3451           
=======================================
  Hits        27785    27785           
  Misses       3204     3204           
  Partials      822      822

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 0134338...c6bbada. Read the comment docs.

@albayenes

This comment has been minimized.

Copy link
Contributor

albayenes commented Aug 3, 2018

Rebased @skoudoro

@arokem arokem merged commit 393350c into nipy:master Aug 3, 2018

3 checks passed

codecov/patch Coverage not affected when comparing 0134338...c6bbada
Details
codecov/project 87.34% remains the same compared to 0134338
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@arokem

This comment has been minimized.

Copy link
Member

arokem commented Aug 3, 2018

Thanks!

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Merge pull request nipy#1560 from albayenes/basic-traking-removed-tra…
…cking-affine-to-visualize-trakvis-correct

Removed affine matrices from tracking.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment