Skip to content
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

[WIP] Add surface.py for tract <-> surface functionality #902

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sambjohnson
Copy link

Add a WIP surface.py to AFQ as a catch-all for new surface functionality, to be fully integrated later. As of its creation, the file only includes functions for .trk to surface representations, used e.g., to create suface-based "endpoint maps."

@pep8speaks
Copy link

pep8speaks commented Aug 30, 2022

Hello @sambjohnson! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 19:53: E231 missing whitespace after ','
Line 19:62: E231 missing whitespace after ','
Line 21:53: E231 missing whitespace after ','
Line 21:62: E231 missing whitespace after ','
Line 23:48: E231 missing whitespace after ','
Line 23:58: E231 missing whitespace after ','
Line 38:35: E231 missing whitespace after ','
Line 38:44: E231 missing whitespace after ','
Line 45:63: E231 missing whitespace after ','
Line 72:35: E231 missing whitespace after ','
Line 72:37: E231 missing whitespace after ','
Line 72:48: E231 missing whitespace after ','
Line 72:51: E231 missing whitespace after ','
Line 72:80: E501 line too long (107 > 79 characters)
Line 72:83: E231 missing whitespace after ','
Line 73:50: E225 missing whitespace around operator
Line 78:50: E225 missing whitespace around operator
Line 80:56: E225 missing whitespace around operator
Line 106:80: E501 line too long (80 > 79 characters)
Line 114:80: E501 line too long (80 > 79 characters)
Line 116:80: E501 line too long (80 > 79 characters)
Line 127:80: E501 line too long (80 > 79 characters)
Line 135:80: E501 line too long (80 > 79 characters)
Line 155:80: E501 line too long (80 > 79 characters)
Line 207:1: E302 expected 2 blank lines, found 1
Line 208:23: E401 multiple imports on one line
Line 213:1: W391 blank line at end of file

Comment last updated at 2022-08-30 20:33:21 UTC

@sambjohnson
Copy link
Author

Note: there is a potential outstanding issue regarding (mis)alignment of tract (.trk) coordinates and surface coordinates. If these two coordinate spaces are not identical, then the translation logic will fail. To this end, here are comments from Noah Benson regarding this issue and how / when to correct for it, which may be helpful for completing this PR.

  • "For all FreeSurfer subjects: FreeSurfer explicitly wants to operate on the original (i.e., not interpolated) values in the T1, so recon-all will reslice the image into an LIA orientation, but it won't align the brain to MNI or anything like that. When recon-all runs, it converts the input image to a "conformed" LIA slicing that all of its operations and output image files are always in (let's call this "freesurfer" space); when FreeSurfer creates the surfaces, it ignores the affine in the input T1 entirely, treats the center of the "freesurfer" image space as the origin, and uses a standard affine for voxel-to-RAS (with just the last column of the affine, corresponding to the image center, being different). TL;DR—for a conformed T1 with shape (rows, cols, slices), this is called the "vox2ras-tkr" transform and is always [-1 0 0 rows/2; 0 0 1 -slices/2; 0 -1 0 cols/2; 0 0 0 1] (assuming 1mm voxels). You can get it from a nibabel MGHImage object via img.header.get_vox2ras_tkr(). However, the affine stored in the original recon-all input file (the "scanner"-space affine) can be anything, and the affine in the other FreeSurfer mgz files like brain.mgz (the "freesurfer"-space affine) will be an LIA version of that scanner-space affine. To be honest, I'm not sure that the freesurfer space is aligned to anything rather than just keeping the same transform as the scanner space, just resliced to LIA, but the point is that the the "freesurfer" space (in the brain.mgz affine) is not the same as FreeSurfer's RAS/surface space, and this is true for all FreeSurfer subjects. If you have coordinates in the "freesurfer" space, then you can convert them into voxel indices using the inverse of the brain.mgz affine then convert them into RAS using the vox2ras-tkr matrix.
  • For all HCP subjects: The HCPpipelines align everything to an MNI template so all subjects have brains that are aligned, and also, all of the image and surface files are in spaces aligned by the transforms just like you would expect. So the affine in an image file converts from voxel indices to surface-RAS."

@arokem arokem added this to the pyAFQ 2.0 milestone Sep 8, 2023
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.

None yet

3 participants