13204: Fix label border rendering issue on flat brain surfaces #13219

scrharsh · 2025-04-17T17:00:49Z

Reference issue

What does this implement/fix?

This pull request addresses an issue where label borders could not be rendered correctly on flat brain surfaces during 3D plotting. Attempting to render borders in this context previously led to errors or misleading visuals.

Changes include:

A conditional check to detect flat surfaces (is_flat).
Emitting a warning when borders=True on flat surfaces:
"Label borders cannot be displayed on flat surfaces. Skipping borders."
Skipping border rendering entirely if the surface is flat.
Added a test to ensure the warning is correctly raised and handled.

Additional information

This fix improves visualization reliability and provides informative user feedback when label borders are incompatible with flat surface rendering.

for more information, see https://pre-commit.ci

wmvanvliet · 2025-04-18T12:21:29Z

There seem to be other changed mixed in with the PR.

scrharsh · 2025-04-18T13:39:52Z

There seem to be other changed mixed in with the PR.

Thanks for pointing that out! Those changes weren’t meant to be part of this PR — I’ll clean it up and push an update shortly.

for more information, see https://pre-commit.ci

scrharsh

All the required changes are done.

for more information, see https://pre-commit.ci

drammock · 2025-04-18T14:26:06Z

test_output.edf

remove this file. If a file is needed for testing, we typically either create it on the fly, or incorporate it into https://github.com/mne-tools/mne-testing-data

Can you please guide me about this?

drammock · 2025-04-18T14:27:54Z

mne/viz/_brain/_brain.py

+        from warnings import warn
+
+        import numpy as np


imports must go at top of file (or if they need to be nested inside funcs/classes to avoid circular import: then at the top of the func/class def).

Please install our pre-commit hooks, they would have caught this (and other) style errors.

I have commit the changes suggested.

drammock · 2025-04-18T14:28:41Z

mne/tests/test_label_borders.py

+            w[-1].message
+        )
+
+    print("Test passed!")


no print statements in tests please.

drammock · 2025-04-18T14:29:00Z

mne/tests/test_label_borders.py

+# Run the test
+test_label_borders()


don't call test functions directly; pytest does that for us.

drammock · 2025-04-18T14:31:38Z

mne/tests/test_label_borders.py

+    subjects_dir = os.path.expanduser(
+        "~/mne_data/MNE-sample-data/subjects"
+    )  # Adjust the path as needed


in general, don't assume the data folder is in ~; use mne.get_config("MNE_DATA").

But for built-in datasets there is the preferable mne.datasets.sample.data_path()

drammock · 2025-04-18T14:32:50Z

mne/tests/test_label_borders.py

+                    warnings.warn(
+                        "Label borders cannot be displayed on flat surfaces. Skipping borders."
+                    )


in general, don't raise warnings inside a test function.

drammock · 2025-04-18T14:34:18Z

mne/viz/_brain/_brain.py

+                warn(
+                    "Label borders cannot be displayed on flat surfaces. Skipping borders."
+                )
+                borders = False


I think we'd prefer to actually make it work to show borders on flat brains, rather than just warning that it's not possible!

for more information, see https://pre-commit.ci

scrharsh · 2025-04-18T18:07:21Z

@larsoner please have a look.

drammock

Please, if you want another review, first post a screenshot of a plot of a flat brain that shows the label borders looking as they ought to.

drammock · 2025-04-18T18:24:27Z

mne/viz/_brain/_brain.py

@@ -10,6 +10,7 @@
 import warnings
 from functools import partial
 from io import BytesIO
+from warnings import warn


revert. we have our own internal warn function (and logger) in mne.utils

drammock · 2025-04-18T18:25:01Z

mne/viz/_brain/_brain.py

@@ -51,7 +52,6 @@
    logger,
    use_log_level,
    verbose,
-    warn,


drammock · 2025-04-18T18:27:24Z

mne/viz/_brain/_brain.py

+            if is_flat:
+                # Instead of warning, calculate and show the borders for flat surfaces
+                keep_idx = _mesh_borders(self.geo[hemi].faces, scalars)
+                show = np.zeros(scalars.size, dtype=np.int64)
+                if isinstance(borders, int):
+                    for _ in range(borders):
+                        # Refine border calculation by checking neighboring borders
+                        keep_idx = np.isin(self.geo[hemi].faces.ravel(), keep_idx)
+                        keep_idx.shape = self.geo[hemi].faces.shape
+                        keep_idx = self.geo[hemi].faces[np.any(keep_idx, axis=1)]
+                        keep_idx = np.unique(keep_idx)
+                show[keep_idx] = 1
+                scalars *= show  # Apply the border filter to the scalars
+
+            else:
+                # For non-flat surfaces, proceed with the existing logic
+                keep_idx = _mesh_borders(self.geo[hemi].faces, scalars)
+                show = np.zeros(scalars.size, dtype=np.int64)
+                if isinstance(borders, int):
+                    for _ in range(borders):
+                        keep_idx = np.isin(self.geo[hemi].faces.ravel(), keep_idx)
+                        keep_idx.shape = self.geo[hemi].faces.shape
+                        keep_idx = self.geo[hemi].faces[np.any(keep_idx, axis=1)]
+                        keep_idx = np.unique(keep_idx)
+                show[keep_idx] = 1
+                scalars *= show


these if and else blocks are identical. How does this solve the problem?

drammock · 2025-04-18T18:29:15Z

mne/tests/test_label_borders.py

+    result = brain.add_label(label, borders=borders)
+    assert expected in result
+
+    # Use internal projection and rendering functions to avoid vulture error


there's a vulture allowlist for genuine false-positives tools/vulture_allowlist.py

scrharsh added 2 commits April 9, 2025 19:19

test failures with scipy 1.15.0 - sph_harm deprecation bug fixed

b5acfa9

Showing label borders is not possible on the flat brain surface

8ef14b5

scrharsh requested review from larsoner, drammock, wmvanvliet, adam2392, mscheltienne, alexrockhill, sappelhoff, cbrnr, agramfort and dengemann as code owners April 17, 2025 17:00

[pre-commit.ci] auto fixes from pre-commit.com hooks

4026798

for more information, see https://pre-commit.ci

scrharsh and others added 13 commits April 18, 2025 19:13

Update _edf.py

9087c45

[pre-commit.ci] auto fixes from pre-commit.com hooks

41e62aa

for more information, see https://pre-commit.ci

Rename __init___1.py to __init___.py

becdb73

Update pick.py

fa7b0d8

Update _edf.py

f4d4a53

Update test_maxwell.py

05cb361

Update test_maxwell.py

335cbe7

Rename __init___.py to __init__.py

f1e404d

Delete mne/tests/test_tfr.py

221a4c0

Update tfr.py

0a66f07

Update tfr.py

093f127

Update transforms.py

f110d7c

Update _brain.py

5bfdd68

scrharsh commented Apr 18, 2025

View reviewed changes

Update test_label_borders.py

9929701

[pre-commit.ci] auto fixes from pre-commit.com hooks

83466da

for more information, see https://pre-commit.ci

drammock requested changes Apr 18, 2025

View reviewed changes

scrharsh and others added 6 commits April 18, 2025 20:05

Update _brain.py

2c7b564

Update _brain.py

6166d8b

Update test_label_borders.py

3e1fd5c

[pre-commit.ci] auto fixes from pre-commit.com hooks

dee8cf3

for more information, see https://pre-commit.ci

Update test_label_borders.py

65ad5cd

[pre-commit.ci] auto fixes from pre-commit.com hooks

9b2f1c3

for more information, see https://pre-commit.ci

scrharsh requested a review from drammock April 18, 2025 15:06

scrharsh and others added 6 commits April 18, 2025 21:21

Update test_label_borders.py

58c4fb0

[pre-commit.ci] auto fixes from pre-commit.com hooks

8e2a106

for more information, see https://pre-commit.ci

Update _brain.py

0c2c00c

Update test_label_borders.py

fb12fc8

Merge branch 'main' into scrharsh/13204

8d9dc1a

Update test_label_borders.py

518b61c

drammock requested changes Apr 18, 2025

View reviewed changes

scrharsh and others added 3 commits April 19, 2025 17:16

Update _brain.py

91c19a8

Update _brain.py

710561a

[autofix.ci] apply automated fixes

cc26434

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

13204: Fix label border rendering issue on flat brain surfaces #13219

13204: Fix label border rendering issue on flat brain surfaces #13219

scrharsh commented Apr 17, 2025

wmvanvliet commented Apr 18, 2025

scrharsh commented Apr 18, 2025

scrharsh left a comment

drammock Apr 18, 2025

scrharsh Apr 18, 2025

drammock Apr 18, 2025

scrharsh Apr 18, 2025

drammock Apr 18, 2025

drammock Apr 18, 2025

drammock Apr 18, 2025

drammock Apr 18, 2025

drammock Apr 18, 2025

scrharsh commented Apr 18, 2025

drammock left a comment

drammock Apr 18, 2025

drammock Apr 18, 2025

drammock Apr 18, 2025

drammock Apr 18, 2025

13204: Fix label border rendering issue on flat brain surfaces #13219

Are you sure you want to change the base?

13204: Fix label border rendering issue on flat brain surfaces #13219

Conversation

scrharsh commented Apr 17, 2025

Reference issue

What does this implement/fix?

Additional information

wmvanvliet commented Apr 18, 2025

scrharsh commented Apr 18, 2025

scrharsh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scrharsh commented Apr 18, 2025

drammock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment