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, ENH: Add sensors to Brain #8803

Closed

Conversation

GuillaumeFavelier
Copy link
Contributor

This PR follows #8382 (comment) and implements add_sensors to plot MEG and EEG sensors.

For now, I move code and experiment. Refactoring is necessary to avoid duplication between plot_alignment() and add_sensors.

This is still work in progress.

@GuillaumeFavelier
Copy link
Contributor Author

I play with it using:

import os.path as op

import numpy as np
import nibabel as nib

import mne

data_path = mne.datasets.sample.data_path()
subjects_dir = op.join(data_path, 'subjects')
raw_fname = op.join(data_path, 'MEG', 'sample', 'sample_audvis_raw.fif')
trans_fname = op.join(data_path, 'MEG', 'sample',
                      'sample_audvis_raw-trans.fif')
raw = mne.io.read_raw_fif(raw_fname)
trans = mne.read_trans(trans_fname)
src = mne.read_source_spaces(op.join(subjects_dir, 'sample', 'bem',
                                     'sample-oct-6-src.fif'))

# Load the T1 file and change the header information to the correct units
t1w = nib.load(op.join(data_path, 'subjects', 'sample', 'mri', 'T1.mgz'))
t1w = nib.Nifti1Image(t1w.dataobj, t1w.affine)
t1w.header['xyzt_units'] = np.array(10, dtype='uint8')
t1_mgh = nib.MGHImage(t1w.dataobj, t1w.affine)

Brain = mne.viz.get_brain_class()
brain = Brain(subject_id="sample", hemi="both", surf="white", alpha=0.7,
              units="m")
brain.add_sensors(info=raw.info, trans=trans, coord_frame='meg')

mne.viz.plot_alignment(raw.info, trans=trans, subject='sample',
                       subjects_dir=subjects_dir, surfaces='head-dense',
                       show_axes=True, dig=True, eeg=[], meg='sensors',
                       coord_frame='meg', mri_fiducials='estimated',
                       fig=brain._renderer.scene())

I had to change units to make the brain scale correctly but I still think some transformation is missing.

image

@larsoner
Copy link
Member

brain.add_sensors should not have a coord_frame argument. It's always MRI because brain is always in MRI. This is probably our transform problem

@GuillaumeFavelier
Copy link
Contributor Author

Working on EEG now.

Where to fetch the sensors' color?

@larsoner
Copy link
Member

Same place as plot_alignment, it's some defaults.py / DEFAULTS entry

https://github.com/mne-tools/mne-python/blob/main/mne/viz/_3d.py#L1063-L1065

Eventually we'll want to add a parameter to allow controlling the sensor color on a channel-name basis, but not in the first PR (need to sort out the API). But if you can at least consider this use case it might help guide the initial design.

@GuillaumeFavelier
Copy link
Contributor Author

Another naive question, should I add support for projected EEG also? On what surface?

Also %(trans)s is not automagically detected in the docstring.

@larsoner
Copy link
Member

Another naive question, should I add support for projected EEG also? On what surface?

Not in the first PR, we can always add it later.

Also %(trans)s is not automagically detected in the docstring.

You either need @verbose decorator if there is a verbose=None kwarg or @fill_doc otherwise

@GuillaumeFavelier
Copy link
Contributor Author

Based on plot_ecog, I got something like the following in this early version:

image

This slider should fit somewhere nice when time_viewer=True, the default pyvista colormap is used and the range of scalar values is calculated on the fly. Glyph refactor is also required for this.

return dict(rr=meg_rrs, tris=meg_tris)


def _get_coord_frame_trans(coord_frame, info, trans):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems confusing, could you just transform to head if in "meg", then transform to mri if in originally in "meg" (now "head" because it was just transformed) or "head"? That seems like it might be more straightforward

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I guess you might need them multiple times so that might not work

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.

3 participants