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

ENH: sEEG and ECoG electrodes should have sensor labels in brain visualizations #12475

Open
libertyh opened this issue Mar 1, 2024 · 14 comments
Labels

Comments

@libertyh
Copy link
Contributor

libertyh commented Mar 1, 2024

Describe the new feature or enhancement

Since sEEG and ECoG montages differ substantially from participant to participant and there is no "default" montage like in EEG, we should have a way to show the labels for each electrode in the mne.viz.Brain. For example, even electrodes within the same brain area can have wildly different names (in our lab, sometimes the electrode RPPST might be in the same area as one named PST-PH or PTG-HG, so it's anyone's guess where they actually are unless you label them). Having the option to include the sensor labels in add_sensors would be very helpful for interpreting evoked activity or analyses on individual electrodes.

Describe your proposed implementation

in brain.add_sensors() in mne.viz.Brain, I propose that sensor_labels be added as a boolean argument, where the labels would be taken directly from info['ch_names']. I'm not sure the best way to implement this based on the 3D viewer, since ideally these would be labels that are not added like the title text but instead would follow the electrode in 3D and be tied to the x, y, z coordinate of each electrode sphere (offset by a tiny amount so the electrode itself is still viewable, and maybe ideally with some control over the font size and characteristics).

In spirit, this is kind of like how in mne.viz.plot_montage you can have the show_names=True argument.

Example for a grid shown below:
image

and for sEEG:
image

For the user, this is helpful when interpreting analyses, and I wouldn't necessarily include all the electrode labels like this in a paper figure, but it's really helpful when exploring the data.

Describe possible alternatives

sensor_labels could also be passed as a list if the user wanted to manually specify their names for some reason (maybe they want to have shorter names or want to name them by function?), but I think this is probably a less realistic usage. Open to opinions.

Additional context

No response

@libertyh libertyh added the ENH label Mar 1, 2024
@libertyh
Copy link
Contributor Author

libertyh commented Mar 1, 2024

Adding @alexrockhill and @agramfort in case you have any thoughts about this, I'm happy to try to implement.

@libertyh libertyh changed the title sEEG and ECoG electrodes should have sensor labels in brain visualizations ENH: sEEG and ECoG electrodes should have sensor labels in brain visualizations Mar 1, 2024
@alexrockhill
Copy link
Contributor

Agreed, this is a good idea! At one point not too long ago, I unified the code to add sensors in mne.viz.plot_alignment with mne.viz.Brain that was great but then we have to either decide to keep those synchronized or, I think probably easier and better, to go in and add things post-rendering like labels. I think it's better because mne.viz.plot_alignment takes a ton of arguments and it's just super confusing to keep adding more vs mne.viz.Brain.add_sensors is much more modular so I think it's a better place to keyword arguments like this. That was the reason that the sensor size argument wasn't added either, because it would then have to go to the shared internal function of mne.viz.plot_alignment and mne.viz.Brain.add_sensors but we can do that too, again, probably not something that would make sense to go into mne.viz.plot_alignment.

@alexrockhill
Copy link
Contributor

Maybe @larsoner knows but I looked into it and couldn't see any function in pyvista to have text at a 3D location and not just a 2D location in the viewport (in the later case if you rotate, it won't be at the right location anymore). If that's not possible, you can label them with mne.viz.snapshot_brain_montage but that would only work for still images. You could add the text in the right locations in 3D for the initial view but I'd worry that those would be confusing when the view rotates and they get out of place. They could be redrawn every time the camera moves, that might make it a bit laggy but maybe not.

@larsoner
Copy link
Member

larsoner commented Mar 1, 2024

Sounds like you want vtkFollower. pyvista just wraps vtk so when needed you can just use vtk objects directly.

@larsoner
Copy link
Member

larsoner commented Mar 1, 2024

... and maybe this VTK example.

@alexrockhill
Copy link
Contributor

Nice! Glad I asked, I tried to check vtk too but I wasn't sure what to search. Maybe a nice pyvista PR to add that or we could just add it in mne first.

@larsoner
Copy link
Member

larsoner commented Mar 1, 2024

PyVista is in more maintenance mode nowadays than trying to add more stuff. I actually think vedo could be a better way to go at some point

@libertyh
Copy link
Contributor Author

libertyh commented Mar 2, 2024

There's also this python VTK example of "billboard text" (I didn't know this, but apparently that refers to text that follows the camera). Can we use vtkmodules ? It seems like it's already installed with MNE so that might be a nice way to go.

@larsoner
Copy link
Member

larsoner commented Mar 2, 2024

Yes vtkBillboardTextActor3D looks even better! You can use vtkmodules, it's what pyvista uses already 👍

@alexrockhill
Copy link
Contributor

PyVista is in more maintenance mode nowadays than trying to add more stuff. I actually think vedo could be a better way to go at some point

Thoughts on fury as a backend? Looks like another wrapper of VTK

@larsoner
Copy link
Member

Based on a quick look vedo looks more complete but who knows

@libertyh
Copy link
Contributor Author

libertyh commented Mar 11, 2024 via email

@larsoner
Copy link
Member

Yes don't switch as part of fixing this issue

@alexrockhill
Copy link
Contributor

Sorry a bit off topic from the main thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants