Create atlas and coordinate_space #215
Conversation
Created structure for loading different labels based on coordinate space and atlas. Still need to construct num2region dictionaries from relevant data
While retaining custom labels for specific scenarios
Making the existing brain functions play nice with the new refactor
gavinmischler
left a comment
gavinmischler
left a comment
There was a problem hiding this comment.
Looks good! Just a couple little comments
naplib/localization/freesurfer.py
Outdated
| surf_type: str = "pial", | ||
| subject: str = "fsaverage", | ||
| coordinate_space: str = 'FSAverage', | ||
| atlas: str = '', |
There was a problem hiding this comment.
Can you change this to default to None rather than empty string?
naplib/localization/freesurfer.py
Outdated
| self.coordinate_space = coordinate_space | ||
| self.atlas = atlas |
There was a problem hiding this comment.
Is there input validation we can do on atlas?
There was a problem hiding this comment.
Changed to check that atlas is either "Destrieux" or "Desikan-Killiany" or SUBJECT_DIR/SUBJECT/label/{hemi}.{atlas}.annot exists
naplib/localization/freesurfer.py
Outdated
| Subject to use, must be a directory within ``subject_dir`` | ||
| coordinate_space : str, default='FSAverage' | ||
| Coordinate space of brain vertices. Must be 'FSAverage' or 'MNI152' | ||
| atlas : str, default='' |
There was a problem hiding this comment.
can you also make this one default to None?
|
Still getting a little bit of unexpected behavior with |
vinaysraghavan
changed the title
|
It seems like the Hemisphere properties self.has_overlay and self.has_overlay_cells are never used. Setting self.has_overlay_cells in paint_overlay() requires calling self.zones(), which is considerably more computationally intensive than simply setting self.overlay[self.labels==self.label2num[label]] = value. Is there any reason to keep self.has_overlay and self.has_overlay_cells? |
Hmm I guess maybe not. I can't remember why we have those now to be honest. |
Merge overlay values within each atlas parcel into a single value. Meant to be used after interpolate_electrodes_onto_brain()
gavinmischler
left a comment
gavinmischler
left a comment
There was a problem hiding this comment.
Just a little docstring thing but looks great!
naplib/localization/freesurfer.py
Outdated
| Parameters | ||
| ---------- | ||
| merge_func : callable, optional |
There was a problem hiding this comment.
I think it's better to say "callable, default=numpy.mean", because optional means that you can pass None and it will be fine, which is not the case
Reference Issues/PRs
See #214
What does this implement/fix? Explain your changes.
Fixes the distinction between coordinate spaces and brain atlases in the Brain object. Added the ability to specify atlas parcellation, including loading custom parcellations stored as .annot files.
Any other comments?
Should not break any prior functionality or use with loading MNI152 vs FreeSurfer and using the custom DK .annot file for the MNI152 data