Interactive version of plot_evoked_fieldmap#11942
Conversation
|
While digging in the whole playback business I stumbled across the |
|
Feel free to delete it if it's not used, it's in |
|
Ready from my end! |
|
CircleCI failed, feel free to look or I can look later |
|
CIs should be fixed, will test and look at code hopefully tomorrow! |
larsoner
left a comment
There was a problem hiding this comment.
When I run the example and slide the Brain time slider I get updates to the clim sliders that are incorrect (they change which they shouldn't, and they don't update the clim values actually), can you replicate?
But other than that it's remarkably smooth!
mne/viz/_3d.py
Outdated
| .. versionadded:: 1.1 | ||
| time_viewer : bool | str | ||
| Display time viewer GUI. Can also be ``"auto"``, which will mean | ||
| ``True`` for the PyVista backend and ``False`` otherwise. |
There was a problem hiding this comment.
PyVista is currently the only 3d backend, I thought at first you meant PyVistaQt vs notebook. But looking at the code I think you mean
| ``True`` for the PyVista backend and ``False`` otherwise. | |
| ``True`` if there is more than one time point and ``False`` otherwise. |
mne/viz/evoked_field.py
Outdated
| .. versionadded:: 1.1 | ||
| time_viewer : bool | str | ||
| Display time viewer GUI. Can also be ``"auto"``, which will mean | ||
| ``True`` for the PyVista backend and ``False`` otherwise. |
mne/viz/evoked_field.py
Outdated
| if not self._in_brain_figure: | ||
| self._renderer.set_camera(azimuth=10, elevation=60) | ||
|
|
||
| self._renderer.show() |
There was a problem hiding this comment.
probably also should be indented / only done if not in a brain?
mne/viz/evoked_field.py
Outdated
| else: | ||
| raise ValueError(f"No {type.upper()} field map currently shown.") | ||
|
|
||
| def rescale(self): |
There was a problem hiding this comment.
yeah I guess so.
mne/viz/tests/test_3d.py
Outdated
|
|
||
| # Test plotting inside an existing Brain figure. This should not work in a notebook. | ||
| brain = Brain("fsaverage", "lh", "inflated", subjects_dir=subjects_dir) | ||
| if get_3d_backend() == "notebook": |
There was a problem hiding this comment.
I thought that renderer ran both notebook and non-notebook variants, but it doesn't look that way :(
Looks like we'd have to create a test_notebook.py like this to test this stuff properly on notebook, basically each "test" contains Python code that must be self-contained (with all imports and variables etc.) necessary to run it in nbexec:
https://github.com/mne-tools/mne-python/blob/main/mne/viz/_brain/tests/test_notebook.py
One fairly low-effort solution is to write some minimal code to take care of covering the time_viewer=True and time_viewer=False cases plus the Brain-embedding case (which would cover this NotImplementedError. If you're sick of this notebook business I could push it quickly I think.
There was a problem hiding this comment.
I'm giving it a try.
Unfortunately, this doesn't replicate on my Window nor my Linux machine. Is this on OSX? |
I was, but after updating I no longer see this behavior. Not sure if you fixed something or I had some broken code, but I think we're okay now! CircleCI and pip-pre are being fixed in #11926 which I'll merge into this branch once it's in. I'll also look at the code again and mark for green when merge assuming I don't find anything else, thanks in advance! |
* upstream/main: Interactive version of plot_evoked_fieldmap (mne-tools#11942) ENH: Add support for Artinis SNIRF data (mne-tools#11926)
|
@wmvanvliet can you quickly fix the failures here? If not I can do it. I think we maybe just need |
Continuation of #11676 but now on top of the new UI events system. This PR is built on top of #11891
All the functionality is there I think. Now it just needs to be made a bit more pretty.
Todo:
rendererso it can be shared betweenBrainandEvokedField