From 15519ebafaf0344a15af5f5f9f56b89bf36d803e Mon Sep 17 00:00:00 2001 From: Marijn van Vliet Date: Tue, 4 Feb 2025 10:02:52 +0200 Subject: [PATCH 01/12] Take units (m or mm) into account when showing fieldmaps on top of brain figures --- mne/viz/evoked_field.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mne/viz/evoked_field.py b/mne/viz/evoked_field.py index cf5a9996216..b1df34c907e 100644 --- a/mne/viz/evoked_field.py +++ b/mne/viz/evoked_field.py @@ -7,6 +7,7 @@ # License: BSD-3-Clause # Copyright the MNE-Python contributors. +from copy import deepcopy from functools import partial import numpy as np @@ -185,6 +186,7 @@ def __init__( if isinstance(fig, Brain): self._renderer = fig._renderer self._in_brain_figure = True + self._units = fig._units if _get_3d_backend() == "notebook": raise NotImplementedError( "Plotting on top of an existing Brain figure " @@ -195,6 +197,7 @@ def __init__( fig, bgcolor=(0.0, 0.0, 0.0), size=(600, 600) ) self._in_brain_figure = False + self._units = "m" self.plotter = self._renderer.plotter self.interaction = interaction @@ -276,8 +279,8 @@ def _prepare_surf_map(self, surf_map, color, alpha): current_data = data_interp(self._current_time) # Make a solid surface - surf = surf_map["surf"] - if self._in_brain_figure: + surf = deepcopy(surf_map["surf"]) + if self._units == "mm": surf["rr"] *= 1000 map_vmax = self._vmax.get(surf_map["kind"]) if map_vmax is None: From 5108607b2be60cebbe7aa8807da03d25f4d6782f Mon Sep 17 00:00:00 2001 From: Marijn van Vliet Date: Tue, 4 Feb 2025 10:27:32 +0200 Subject: [PATCH 02/12] Add unit test and towncrier --- doc/changes/devel/13101.bugfix.rst | 1 + mne/viz/tests/test_3d.py | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 doc/changes/devel/13101.bugfix.rst diff --git a/doc/changes/devel/13101.bugfix.rst b/doc/changes/devel/13101.bugfix.rst new file mode 100644 index 00000000000..d24e55b5056 --- /dev/null +++ b/doc/changes/devel/13101.bugfix.rst @@ -0,0 +1 @@ +Take units (m or mm) into account when drawing :func:`~mne.viz.plot_evoked_field` on top of :class:`~mne.viz.Brain`, by `Marijn van Vliet`_. diff --git a/mne/viz/tests/test_3d.py b/mne/viz/tests/test_3d.py index 34022d59768..e3e4a2143d2 100644 --- a/mne/viz/tests/test_3d.py +++ b/mne/viz/tests/test_3d.py @@ -192,9 +192,18 @@ def test_plot_evoked_field(renderer): ) evoked.plot_field(maps, time=0.1, n_contours=n_contours) - # Test plotting inside an existing Brain figure. - brain = Brain("fsaverage", "lh", "inflated", subjects_dir=subjects_dir) - fig = evoked.plot_field(maps, time=0.1, fig=brain) + # Test plotting inside an existing Brain figure. Check that units are taken into + # account. + for units in ["mm", "m"]: + brain = Brain( + "fsaverage", "lh", "inflated", units=units, subjects_dir=subjects_dir + ) + fig = evoked.plot_field(maps, time=0.1, fig=brain) + assert brain._units == fig._units + scale = 1000 if units == "mm" else 1 + assert ( + fig._surf_maps[0]["surf"]["rr"][0, 0] == scale * maps[0]["surf"]["rr"][0, 0] + ) # Test some methods fig = evoked.plot_field(maps, time_viewer=True) From e243310678b98c30d1044e2e571f439f5a632e43 Mon Sep 17 00:00:00 2001 From: Marijn van Vliet Date: Tue, 4 Feb 2025 10:29:32 +0200 Subject: [PATCH 03/12] Don't make unnecessary copy --- mne/viz/evoked_field.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne/viz/evoked_field.py b/mne/viz/evoked_field.py index b1df34c907e..2138f800136 100644 --- a/mne/viz/evoked_field.py +++ b/mne/viz/evoked_field.py @@ -279,8 +279,8 @@ def _prepare_surf_map(self, surf_map, color, alpha): current_data = data_interp(self._current_time) # Make a solid surface - surf = deepcopy(surf_map["surf"]) if self._units == "mm": + surf = deepcopy(surf_map["surf"]) surf["rr"] *= 1000 map_vmax = self._vmax.get(surf_map["kind"]) if map_vmax is None: From a1f5328cfa3cf368d6e7af82b18782a1ddb0ae39 Mon Sep 17 00:00:00 2001 From: Marijn van Vliet Date: Tue, 4 Feb 2025 10:30:33 +0200 Subject: [PATCH 04/12] Fix --- mne/viz/evoked_field.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mne/viz/evoked_field.py b/mne/viz/evoked_field.py index 2138f800136..839259ee117 100644 --- a/mne/viz/evoked_field.py +++ b/mne/viz/evoked_field.py @@ -279,8 +279,9 @@ def _prepare_surf_map(self, surf_map, color, alpha): current_data = data_interp(self._current_time) # Make a solid surface + surf = surf_map["surf"] if self._units == "mm": - surf = deepcopy(surf_map["surf"]) + surf = deepcopy(surf) surf["rr"] *= 1000 map_vmax = self._vmax.get(surf_map["kind"]) if map_vmax is None: From cca1c6022ee74e09b35a70239cd82be21cecae57 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 4 Feb 2025 13:05:52 -0500 Subject: [PATCH 05/12] FIX: Ig --- mne/conftest.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mne/conftest.py b/mne/conftest.py index 2a73c7a1b8e..8451c9e776b 100644 --- a/mne/conftest.py +++ b/mne/conftest.py @@ -190,6 +190,8 @@ def pytest_configure(config: pytest.Config): ignore:Starting field name with a underscore.*: # joblib ignore:process .* is multi-threaded, use of fork/exec.*:DeprecationWarning + # sklearn + ignore:Python binding for RankQuantileOptions.*:RuntimeWarning """ # noqa: E501 for warning_line in warning_lines.split("\n"): warning_line = warning_line.strip() From 4d6beae19fe31dc29e2ae6ab1f6f6cadab5f6b9a Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 4 Feb 2025 14:00:28 -0500 Subject: [PATCH 06/12] Update mne/viz/tests/test_3d.py --- mne/viz/tests/test_3d.py | 1 + 1 file changed, 1 insertion(+) diff --git a/mne/viz/tests/test_3d.py b/mne/viz/tests/test_3d.py index e3e4a2143d2..6e2e2eded96 100644 --- a/mne/viz/tests/test_3d.py +++ b/mne/viz/tests/test_3d.py @@ -204,6 +204,7 @@ def test_plot_evoked_field(renderer): assert ( fig._surf_maps[0]["surf"]["rr"][0, 0] == scale * maps[0]["surf"]["rr"][0, 0] ) + brain.close() # Test some methods fig = evoked.plot_field(maps, time_viewer=True) From 855ba1315ab7f9fdbcf7d474e85dcbc510a3320e Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 4 Feb 2025 15:31:31 -0500 Subject: [PATCH 07/12] FIX: Skippy --- mne/viz/tests/test_3d.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/mne/viz/tests/test_3d.py b/mne/viz/tests/test_3d.py index 6e2e2eded96..7af43d8a5f8 100644 --- a/mne/viz/tests/test_3d.py +++ b/mne/viz/tests/test_3d.py @@ -2,6 +2,8 @@ # License: BSD-3-Clause # Copyright the MNE-Python contributors. +import os +import platform from contextlib import nullcontext from pathlib import Path @@ -204,7 +206,6 @@ def test_plot_evoked_field(renderer): assert ( fig._surf_maps[0]["surf"]["rr"][0, 0] == scale * maps[0]["surf"]["rr"][0, 0] ) - brain.close() # Test some methods fig = evoked.plot_field(maps, time_viewer=True) @@ -226,8 +227,14 @@ def test_plot_evoked_field(renderer): assert isinstance(fig, Figure3D) +bad_ci = ( + os.getenv("MNE_CI_KIND", "") in ("conda", "mamba") and platform.system() == "Linux" +) + + @testing.requires_testing_data @pytest.mark.slowtest +@pytest.mark.skipif(bad_ci, reason="Segfaults on Linux conda CI") def test_plot_evoked_field_notebook(renderer_notebook, nbexec): """Test plotting the evoked field inside a notebook.""" import pytest From 899f9306db49bd9630e297ccfcaa57e42fe5667e Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 4 Feb 2025 16:26:56 -0500 Subject: [PATCH 08/12] FIX: Conda --- mne/conftest.py | 13 ++++++++++--- mne/viz/tests/test_3d.py | 8 -------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/mne/conftest.py b/mne/conftest.py index 8451c9e776b..2d57f2fb186 100644 --- a/mne/conftest.py +++ b/mne/conftest.py @@ -6,6 +6,7 @@ import inspect import os import os.path as op +import platform import re import shutil import sys @@ -287,9 +288,10 @@ def __init__(self, exception_handler=None, signals=None): @pytest.fixture(scope="session") def azure_windows(): """Determine if running on Azure Windows.""" - return os.getenv( - "AZURE_CI_WINDOWS", "false" - ).lower() == "true" and sys.platform.startswith("win") + return ( + os.getenv("AZURE_CI_WINDOWS", "false").lower() == "true" + and platform.system() == "Windows" + ) @pytest.fixture(scope="function") @@ -612,6 +614,11 @@ def renderer_pyvistaqt(request, options_3d, garbage_collect): @pytest.fixture(params=[pytest.param("notebook", marks=pytest.mark.pvtest)]) def renderer_notebook(request, options_3d): """Yield the 3D notebook renderer.""" + if ( + os.getenv("MNE_CI_KIND", "") in ("conda", "mamba") + and platform.system() == "Linux" + ): + pytest.skip("Skipping notebook tests on conda Linux CI") with _use_backend(request.param, interactive=False) as renderer: yield renderer diff --git a/mne/viz/tests/test_3d.py b/mne/viz/tests/test_3d.py index 7af43d8a5f8..e3e4a2143d2 100644 --- a/mne/viz/tests/test_3d.py +++ b/mne/viz/tests/test_3d.py @@ -2,8 +2,6 @@ # License: BSD-3-Clause # Copyright the MNE-Python contributors. -import os -import platform from contextlib import nullcontext from pathlib import Path @@ -227,14 +225,8 @@ def test_plot_evoked_field(renderer): assert isinstance(fig, Figure3D) -bad_ci = ( - os.getenv("MNE_CI_KIND", "") in ("conda", "mamba") and platform.system() == "Linux" -) - - @testing.requires_testing_data @pytest.mark.slowtest -@pytest.mark.skipif(bad_ci, reason="Segfaults on Linux conda CI") def test_plot_evoked_field_notebook(renderer_notebook, nbexec): """Test plotting the evoked field inside a notebook.""" import pytest From 671f40bcbef0b596fe1dfad9d32e9af9e8b95134 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Wed, 5 Feb 2025 09:15:45 -0500 Subject: [PATCH 09/12] FIX: Maybe --- mne/viz/tests/test_3d.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mne/viz/tests/test_3d.py b/mne/viz/tests/test_3d.py index e3e4a2143d2..97b10621108 100644 --- a/mne/viz/tests/test_3d.py +++ b/mne/viz/tests/test_3d.py @@ -191,6 +191,7 @@ def test_plot_evoked_field(renderer): ch_type=t, ) evoked.plot_field(maps, time=0.1, n_contours=n_contours) + renderer.backend._close_all() # Test plotting inside an existing Brain figure. Check that units are taken into # account. @@ -204,6 +205,7 @@ def test_plot_evoked_field(renderer): assert ( fig._surf_maps[0]["surf"]["rr"][0, 0] == scale * maps[0]["surf"]["rr"][0, 0] ) + renderer.backend._close_all() # Test some methods fig = evoked.plot_field(maps, time_viewer=True) @@ -223,6 +225,7 @@ def test_plot_evoked_field(renderer): fig = evoked.plot_field(maps, time_viewer=False) assert isinstance(fig, Figure3D) + renderer.backend._close_all() @testing.requires_testing_data From 5e559679d28c4c86d8a89b334a0bf4843657d266 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Wed, 5 Feb 2025 10:16:21 -0500 Subject: [PATCH 10/12] FIX: Try --- .github/workflows/tests.yml | 5 +---- mne/conftest.py | 5 ----- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 298908cdc65..40462bb3517 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -102,10 +102,7 @@ jobs: # Python (if conda) - name: Fixes for conda run: | - # For some reason on Linux we get crashes - if [[ "$RUNNER_OS" == "Linux" ]]; then - sed -i "/numba/d" environment.yml - elif [[ "$RUNNER_OS" == "macOS" ]]; then + if [[ "$RUNNER_OS" == "macOS" ]]; then sed -i "" "s/ - PySide6 .*/ - PySide6 <6.8/g" environment.yml fi if: matrix.kind == 'conda' || matrix.kind == 'mamba' diff --git a/mne/conftest.py b/mne/conftest.py index 2d57f2fb186..fabf9f93e5a 100644 --- a/mne/conftest.py +++ b/mne/conftest.py @@ -614,11 +614,6 @@ def renderer_pyvistaqt(request, options_3d, garbage_collect): @pytest.fixture(params=[pytest.param("notebook", marks=pytest.mark.pvtest)]) def renderer_notebook(request, options_3d): """Yield the 3D notebook renderer.""" - if ( - os.getenv("MNE_CI_KIND", "") in ("conda", "mamba") - and platform.system() == "Linux" - ): - pytest.skip("Skipping notebook tests on conda Linux CI") with _use_backend(request.param, interactive=False) as renderer: yield renderer From aaa25397622c9b9c5510890f157fedd2ddfdea3a Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Wed, 5 Feb 2025 10:43:46 -0500 Subject: [PATCH 11/12] FIX: Revert --- .github/workflows/tests.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 40462bb3517..298908cdc65 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -102,7 +102,10 @@ jobs: # Python (if conda) - name: Fixes for conda run: | - if [[ "$RUNNER_OS" == "macOS" ]]; then + # For some reason on Linux we get crashes + if [[ "$RUNNER_OS" == "Linux" ]]; then + sed -i "/numba/d" environment.yml + elif [[ "$RUNNER_OS" == "macOS" ]]; then sed -i "" "s/ - PySide6 .*/ - PySide6 <6.8/g" environment.yml fi if: matrix.kind == 'conda' || matrix.kind == 'mamba' From 9658410d72ba3407a4927e801745b240e8d166ca Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Wed, 5 Feb 2025 12:03:54 -0500 Subject: [PATCH 12/12] FIX: Cmon --- azure-pipelines.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 7149edac50b..9c42b3286c1 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -89,6 +89,7 @@ stages: DISPLAY: ':99' OPENBLAS_NUM_THREADS: '1' MNE_TEST_ALLOW_SKIP: '^.*(PySide6 causes segfaults).*$' + MNE_BROWSER_PRECOMPUTE: 'false' steps: - bash: | set -e