Open libertyh opened 4 months ago
Adding @alexrockhill and @agramfort in case you have any thoughts about this, I'm happy to try to implement.
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
.
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.
Sounds like you want vtkFollower
. pyvista
just wraps vtk
so when needed you can just use vtk
objects directly.
... and maybe this VTK example.
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.
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
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.
Yes vtkBillboardTextActor3D
looks even better! You can use vtkmodules
, it's what pyvista
uses already 👍
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
Based on a quick look vedo looks more complete but who knows
This would be a pretty large change though, no? I think the vtk billboard text seems fine as a solution for this particular problem. I did look into fury a bit last year at Neurohackademy but I’m not totally sure on a total replacement just for something like this. Will try to add more on this soon, but have some upcoming travel so won’t be able to get to this for a little while. On Mar 11, 2024, at 15:58, Eric Larson @.***> wrote: Based on a quick look vedo looks more complete but who knows
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>
Yes don't switch as part of fixing this issue
Sorry a bit off topic from the main thread
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 inadd_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 thatsensor_labels
be added as a boolean argument, where the labels would be taken directly frominfo['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:
and for sEEG:
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