[MRG, ENH] Find aseg labels near montage #9545

alexrockhill · 2021-07-09T03:06:19Z

The idea here is to find which voxels are nearby an electrode contact in order to know which surfaces to plot and for analysis.

This shares internal functions with #9544 and is a good reason to make mne.surfaces._find_voxel_neighbors private so that it can be used in both scenarios without confusion and user-facing function.

alexrockhill · 2021-07-15T02:22:40Z

@larsoner, I think it might be best to change the default back to aseg.mgz because fsaverage doesn't have any other segmentations in the sample data.

alexrockhill · 2021-07-15T02:34:10Z

This is ready for review too, but this PR has to merge first mne-tools/mne-misc-data#9

larsoner · 2021-07-15T02:56:35Z

Even after fetch_fsaverage is called?

We can also update the sample dataset pretty easily, so I wouldn't let it be a limiting factor

alexrockhill · 2021-07-15T03:09:36Z

Even after fetch_fsaverage is called?

We can also update the sample dataset pretty easily, so I wouldn't let it be a limiting factor

Yes but good to know, maybe we should update that then

alexrockhill · 2021-07-15T03:22:34Z

This also updates the seeg tutorial which was out-of-date because of the new data format

alexrockhill · 2021-07-15T03:23:23Z

Here's what the plots I added look like by the way.

larsoner · 2021-07-15T14:18:29Z

I think it might be best to change the default back to aseg.mgz because fsaverage doesn't have any other segmentations in the sample data.

I just did mne.datasets.sample.data_path(force_update=True, verbose=True) (i.e., not even running fetch_fsaverage) and I see:

$ ls ~/mne_data/MNE-sample-data/subjects/fsaverage/mri
aparc.a2005s+aseg.mgz  aparc+aseg.mgz  brainmask.mgz  lh.ribbon.mgz   orig      p.aseg.mgz     ribbon.mgz        subcort.prob.mgz  transforms
aparc.a2009s+aseg.mgz  aseg.mgz        brain.mgz      mni305.cor.mgz  orig.mgz  rh.ribbon.mgz  subcort.prob.log  T1.mgz

so aparc+aseg.mgz is in there. Maybe your version of sample was outdated?

larsoner

Can anything be refactored with mne.label.find_pos_in_annot?

alexrockhill · 2021-07-15T21:56:00Z

find_pos_in_annot

I didn't see that one, yeah looks like it. I think it might have to be refactored to accept multiple positions though since you wouldn't want to be loading the aseg file over and over.

alexrockhill · 2021-07-15T21:58:38Z

I think it might be best to change the default back to aseg.mgz because fsaverage doesn't have any other segmentations in the sample data.

I just did mne.datasets.sample.data_path(force_update=True, verbose=True) (i.e., not even running fetch_fsaverage) and I see:
$ ls ~/mne_data/MNE-sample-data/subjects/fsaverage/mri
aparc.a2005s+aseg.mgz  aparc+aseg.mgz  brainmask.mgz  lh.ribbon.mgz   orig      p.aseg.mgz     ribbon.mgz        subcort.prob.mgz  transforms
aparc.a2009s+aseg.mgz  aseg.mgz        brain.mgz      mni305.cor.mgz  orig.mgz  rh.ribbon.mgz  subcort.prob.log  T1.mgz
so aparc+aseg.mgz is in there. Maybe your version of sample was outdated?

Yep, mine must have just been way out-of-date.

alexrockhill · 2021-07-16T06:45:33Z

Ok so I changed one thing that I was very unsure about: label.get_pos_in_annot went to label.get_pos_in_aseg which included backdating a function by someone else. Since it was before the next release, I think this might be okay since I think calling it annot is confusing because annot files are something slightly different and already in use to mean a different thing.

Other than that, I tried to refactor to use the function but realized after doing so that it actually would be really inefficient because each channel of the montage is looped over so the aseg would have to be loaded in every time to use that function. I did refactor a bit into a _get_aseg function in _freesurfer.py so that some of the codebase is shared.

If all that is okay, should be good to go.

larsoner · 2021-07-16T18:16:30Z

label.get_pos_in_aseg which included backdating a function by someone else. Since it was before the next release, I think this might be okay since I think calling it annot is confusing because annot files are something slightly different and already in use to mean a different thing.

Agreed this naming is better, I'll look at the refactoring now

larsoner · 2021-07-16T18:17:06Z

Actually I'll wait until #9544 is merged and this is rebased, feel free to ping me once that's done @alexrockhill

alexrockhill · 2021-07-16T20:19:31Z

I think that looks pretty cool and informative

alexrockhill · 2021-07-16T22:16:00Z

label.get_pos_in_aseg which included backdating a function by someone else. Since it was before the next release, I think this might be okay since I think calling it annot is confusing because annot files are something slightly different and already in use to mean a different thing.

Agreed this naming is better, I'll look at the refactoring now

I took that part out and put it in a separate PR, this should be good to go as well after I rebase

larsoner · 2021-07-16T22:17:47Z

I took that part out and put it in a separate PR, this should be good to go as well after I rebase

The refactoring PR should be merged first, then, right?

alexrockhill · 2021-07-16T22:19:15Z

I took that part out and put it in a separate PR, this should be good to go as well after I rebase

The refactoring PR should be merged first, then, right?

They are actually independent/use the exact same _freesurfer.py _get_aseg code from a git checkout aseg -- mne/_freesurfer.py call.

to show Eric revert all Brain API changes, for another PR in progress pare down in progress in progress in progress work in progress but conceptually finished forgot to use the aseg wip wip working version with cool visualization, I think fix tests fix gallery number fix tests use circular layout forgot parens keep channel order, convert to OrderedDict change default fix default wip refactored label, cleaned up ordering of plot, shared some code fix doc fix name error wip remove aseg refactoring shared from label forgot to remove one more refactoring more fixes

alexrockhill · 2021-07-17T14:28:36Z

Looks good to merge by me!

doc/changes/latest.inc

mne/surface.py

mne/viz/circle.py

larsoner · 2021-07-19T13:43:56Z

Thanks @alexrockhill !

* upstream/main: [MRG, ENH] Find aseg labels near montage (mne-tools#9545) Add label to colorbar in GAT plot [skip actions] (mne-tools#9582) [ENH, MRG] Encapsulate warp elec image in function (mne-tools#9544) [DOC, MRG] Add "info" to `docdict` (mne-tools#9574) [MRG] Add `units` parameter to get_data for Evoked (mne-tools#9578) [MRG, ENH] Get annotation description from snirf stim name (mne-tools#9575) [MRG] ENH, FIX: Add tmin/tmax parameters to get_data methods, fix bug in find_bads_ecg (mne-tools#9556)

drammock · 2021-07-19T16:12:03Z

@alexrockhill The doc build failed on this PR, and it looks related, see https://app.circleci.com/pipelines/github/mne-tools/mne-python/9131/workflows/e0f6c14c-27ff-43dd-90c2-939a03863e81/jobs/31255

larsoner · 2021-07-19T16:13:55Z

@alexrockhill can you add a fix to #9540 ? I think that's very close to merge

alexrockhill · 2021-07-19T16:59:57Z

@alexrockhill can you add a fix to #9540 ? I think that's very close to merge

Sure right on it

alexrockhill force-pushed the aseg branch from 52031b0 to 9a170fa Compare July 15, 2021 02:32

alexrockhill changed the title ~~[WIP, ENH] Find aseg labels near montage~~ [MRG, ENH] Find aseg labels near montage Jul 15, 2021

larsoner reviewed Jul 15, 2021

View reviewed changes

alexrockhill force-pushed the aseg branch from b20cd8a to dc9635a Compare July 16, 2021 06:41

alexrockhill mentioned this pull request Jul 16, 2021

[ENH, MRG] Add mne.viz.Brain.add_volume_labels #9540

Merged

larsoner mentioned this pull request Jul 16, 2021

[ENH, MRG] Encapsulate warp elec image in function #9544

Merged

alexrockhill mentioned this pull request Jul 16, 2021

[ENH, MRG] Fix find aseg for Dipole #9579

Merged

alexrockhill force-pushed the aseg branch from 0660c0e to ce9d4e5 Compare July 16, 2021 19:32

alexrockhill force-pushed the aseg branch from 1c5d82f to c43be86 Compare July 16, 2021 22:31

alexrockhill added 3 commits July 16, 2021 15:33

fix spacing

69cdb3a

fix circular import

351c9c2

one more missing space

5b2ee19

agramfort reviewed Jul 18, 2021

View reviewed changes

doc/changes/latest.inc Outdated Show resolved Hide resolved

mne/surface.py Outdated Show resolved Hide resolved

mne/surface.py Outdated Show resolved Hide resolved

mne/viz/circle.py Show resolved Hide resolved

mne/viz/circle.py Show resolved Hide resolved

name change roi->label

3af7877

agramfort approved these changes Jul 18, 2021

View reviewed changes

larsoner merged commit b91e4f7 into mne-tools:main Jul 19, 2021

alexrockhill deleted the aseg branch July 19, 2021 15:35

Uh oh!

[MRG, ENH] Find aseg labels near montage #9545

[MRG, ENH] Find aseg labels near montage #9545

Uh oh!

Conversation

alexrockhill commented Jul 9, 2021

Uh oh!

alexrockhill commented Jul 15, 2021

Uh oh!

alexrockhill commented Jul 15, 2021

Uh oh!

larsoner commented Jul 15, 2021

Uh oh!

alexrockhill commented Jul 15, 2021

Uh oh!

alexrockhill commented Jul 15, 2021

Uh oh!

alexrockhill commented Jul 15, 2021

Uh oh!

larsoner commented Jul 15, 2021

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

alexrockhill commented Jul 15, 2021

Uh oh!

alexrockhill commented Jul 15, 2021

Uh oh!

alexrockhill commented Jul 16, 2021

Uh oh!

larsoner commented Jul 16, 2021

Uh oh!

larsoner commented Jul 16, 2021

Uh oh!

alexrockhill commented Jul 16, 2021

Uh oh!

alexrockhill commented Jul 16, 2021

Uh oh!

larsoner commented Jul 16, 2021

Uh oh!

alexrockhill commented Jul 16, 2021

Uh oh!

alexrockhill commented Jul 17, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

larsoner commented Jul 19, 2021

Uh oh!

drammock commented Jul 19, 2021

Uh oh!

larsoner commented Jul 19, 2021

Uh oh!

alexrockhill commented Jul 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants