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

Reorient tracks according to ROI #751

Merged
merged 6 commits into from Nov 6, 2015

Conversation

Projects
None yet
3 participants
@arokem
Member

arokem commented Oct 28, 2015

This is important in order to correctly pull out a core of a bundle based on a median operation on the coordinates. For details, see:

https://github.com/arokem/AFQ-notebooks/blob/08ac2fc66d1ded0234eec898438d0c7f8380cafc/AFQ-registration-callosum.ipynb

Parameters
----------
streamlines : list
List of 3d arrays. Coordinates of the

This comment has been minimized.

@jchoude

jchoude Oct 29, 2015

Contributor

Missing the end of the comment

Binary masks designating the location of the regions of interest, or
coordinate arrays (3d coordinates)
affine : ndarray
Affine transformation from voxels to streamlines. Default: identity.

This comment has been minimized.

@jchoude

jchoude Oct 29, 2015

Contributor

Since this is something that pops up in almost all of our internal discussion on streamlines: should the affine documentation specify if the offset should be included in the transformation? By offset, I mean the same offset that is used when mapping streamlines to voxel coordinates to interact with a mask, which is normally 0.5 * the voxel size.

I would say that it is up to the user, and in most cases, it probably won't change the end result, but I'm curious what others would say.

@jchoude

This comment has been minimized.

Contributor

jchoude commented Oct 29, 2015

Good, simple work. If there was an approve button, I would click it.

@arokem

This comment has been minimized.

Member

arokem commented Oct 29, 2015

Thanks for taking a look, @jchoude - and thanks for the comments.

I added a couple more tests, to get full coverage, and I added a copy kwarg, so that streamlines can be copied or changed in-place (default is to copy).

Garyfallidis added a commit that referenced this pull request Nov 6, 2015

Merge pull request #751 from arokem/reorient-tracks
Reorient tracks according to ROI

@Garyfallidis Garyfallidis merged commit bda2301 into nipy:master Nov 6, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment