Skip to content

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

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

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
b5acfa9
test failures with scipy 1.15.0 - sph_harm deprecation bug fixed
scrharsh Apr 9, 2025
8ef14b5
Showing label borders is not possible on the flat brain surface
scrharsh Apr 17, 2025
4026798
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 17, 2025
9087c45
Update _edf.py
scrharsh Apr 18, 2025
41e62aa
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 18, 2025
becdb73
Rename __init___1.py to __init___.py
scrharsh Apr 18, 2025
fa7b0d8
Update pick.py
scrharsh Apr 18, 2025
f4d4a53
Update _edf.py
scrharsh Apr 18, 2025
05cb361
Update test_maxwell.py
scrharsh Apr 18, 2025
335cbe7
Update test_maxwell.py
scrharsh Apr 18, 2025
f1e404d
Rename __init___.py to __init__.py
scrharsh Apr 18, 2025
221a4c0
Delete mne/tests/test_tfr.py
scrharsh Apr 18, 2025
0a66f07
Update tfr.py
scrharsh Apr 18, 2025
093f127
Update tfr.py
scrharsh Apr 18, 2025
f110d7c
Update transforms.py
scrharsh Apr 18, 2025
5bfdd68
Update _brain.py
scrharsh Apr 18, 2025
9929701
Update test_label_borders.py
scrharsh Apr 18, 2025
83466da
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 18, 2025
2c7b564
Update _brain.py
scrharsh Apr 18, 2025
6166d8b
Update _brain.py
scrharsh Apr 18, 2025
3e1fd5c
Update test_label_borders.py
scrharsh Apr 18, 2025
dee8cf3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 18, 2025
65ad5cd
Update test_label_borders.py
scrharsh Apr 18, 2025
9b2f1c3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 18, 2025
58c4fb0
Update test_label_borders.py
scrharsh Apr 18, 2025
8e2a106
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Apr 18, 2025
0c2c00c
Update _brain.py
scrharsh Apr 18, 2025
fb12fc8
Update test_label_borders.py
scrharsh Apr 18, 2025
8d9dc1a
Merge branch 'main' into scrharsh/13204
scrharsh Apr 18, 2025
518b61c
Update test_label_borders.py
scrharsh Apr 18, 2025
91c19a8
Update _brain.py
scrharsh Apr 19, 2025
710561a
Update _brain.py
scrharsh Apr 19, 2025
cc26434
[autofix.ci] apply automated fixes
autofix-ci[bot] Apr 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions mne/tests/test_label_borders.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import os
import warnings

import numpy as np

import mne


def test_label_borders():
# Simulate the subjects_dir manually (use a local path)
subjects_dir = os.path.expanduser(
"~/mne_data/MNE-sample-data/subjects"
) # Adjust the path as needed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

subject = "fsaverage" # Use a typical subject name from the dataset

# Create mock labels as if they were read from the annotation file
# Here, we're just using a few dummy labels for testing purposes, adding 'hemi' and 'vertices'
labels = [
mne.Label(np.array([0, 1, 2]), name=f"label_{i}", hemi="lh") for i in range(3)
]

# Create a mock Brain object with a flat surface
class MockBrain:
def __init__(self, subject, hemi, surf):
self.subject = subject
self.hemi = hemi
self.surf = surf

def add_label(self, label, borders=False):
# Simulate adding a label and handling borders logic
if borders:
is_flat = self.surf == "flat"
if is_flat:
warnings.warn(
"Label borders cannot be displayed on flat surfaces. Skipping borders."
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

else:
print(f"Adding borders to label: {label.name}")
else:
print(f"Adding label without borders: {label.name}")

# Create the mock Brain object
brain = MockBrain(subject=subject, hemi="lh", surf="flat")

# Test: Add label with borders (this should show a warning for flat surfaces)
with warnings.catch_warnings(record=True) as w:
brain.add_label(labels[0], borders=True)

# Assert that the warning is triggered for displaying borders on flat surfaces
assert len(w) > 0
assert issubclass(w[-1].category, UserWarning)
assert "Label borders cannot be displayed on flat surfaces" in str(
w[-1].message
)

print("Test passed!")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no print statements in tests please.



# Run the test
test_label_borders()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

46 changes: 36 additions & 10 deletions mne/viz/_brain/_brain.py
Original file line number Diff line number Diff line change
Expand Up @@ -2261,17 +2261,43 @@ def add_label(

scalars = np.zeros(self.geo[hemi].coords.shape[0])
scalars[ids] = 1

from warnings import warn

import numpy as np
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have commit the changes suggested.


is_flat = self._hemi_surfs[hemi]["surface"] == "flat"

if borders:
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
if is_flat:
warn(
"Label borders cannot be displayed on flat surfaces. Skipping borders."
)
borders = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

else:
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

# if borders:
# 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

for _, _, v in self._iter_views(hemi):
mesh = self._layered_meshes[hemi]
mesh.add_overlay(
Expand Down
Binary file added test_output.edf
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please guide me about this?

Binary file not shown.
Loading