-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
base: main
Are you sure you want to change the base?
Changes from 16 commits
b5acfa9
8ef14b5
4026798
9087c45
41e62aa
becdb73
fa7b0d8
f4d4a53
05cb361
335cbe7
f1e404d
221a4c0
0a66f07
093f127
f110d7c
5bfdd68
9929701
83466da
2c7b564
6166d8b
3e1fd5c
dee8cf3
65ad5cd
9b2f1c3
58c4fb0
8e2a106
0c2c00c
fb12fc8
8d9dc1a
518b61c
91c19a8
710561a
cc26434
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
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." | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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!") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no print statements in tests please. |
||
|
||
|
||
# Run the test | ||
test_label_borders() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't call test functions directly; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2261,17 +2261,31 @@ def add_label( | |
|
||
scalars = np.zeros(self.geo[hemi].coords.shape[0]) | ||
scalars[ids] = 1 | ||
|
||
from warnings import warn | ||
|
||
import numpy as np | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
||
for _, _, v in self._iter_views(hemi): | ||
mesh = self._layered_meshes[hemi] | ||
mesh.add_overlay( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please guide me about this? |
There was a problem hiding this comment.
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
~
; usemne.get_config("MNE_DATA")
.But for built-in datasets there is the preferable
mne.datasets.sample.data_path()