[BUG, MRG] Remove check on mne.viz.Brain.add_volume_labels
#11889
Conversation
| @@ -2628,8 +2628,6 @@ def add_volume_labels( | |||
| import nibabel as nib | |||
|
|
|||
| # load anatomical segmentation image | |||
| if not aseg.endswith("aseg"): | |||
There was a problem hiding this comment.
Seems like a safer approach would be:
if not aseg.endswith(("aseg", "wmparc")):
There was a problem hiding this comment.
We don't check this kind of stuff elsewhere i.e. if you try and read a trans file, it doesn't check that it ends with trans.fif so maybe better just to let any readable file
There was a problem hiding this comment.
We don't check this kind of stuff elsewhere
Actually we do -- we have a filename checking function to do this sort of thing for raw/epochs/evoked. Given our tight coupling with FreeSurfer and how infrequently they add new stuff, I don't mind having to change the check every few years to accommodate a new name. In the meantime if we get rid of the check we risk users accidentally passing files that don't work and getting some cryptic error later. There's no telling how many times the current check has saved a user some pain but I bet it's a number greater than zero somewhere...
There was a problem hiding this comment.
Sounds good, okay to change to a warning though?
There was a problem hiding this comment.
Lets default to error and allow overriding with _on_missing style handling
There was a problem hiding this comment.
I just used endswith parc or aseg to be slightly more permissive without adding an extra argument which I'm not sure is worth the added complexity
| @@ -2628,8 +2628,6 @@ def add_volume_labels( | |||
| import nibabel as nib | |||
|
|
|||
| # load anatomical segmentation image | |||
| if not aseg.endswith("aseg"): | |||
There was a problem hiding this comment.
Seems like a safer approach would be:
if not aseg.endswith(("aseg", "wmparc")):
|
Thanks @alexrockhill ! |
* upstream/main: [pre-commit.ci] pre-commit autoupdate (mne-tools#11911) [BUG, MRG] Remove check on `mne.viz.Brain.add_volume_labels` (mne-tools#11889) Small splits fix (mne-tools#11905) adds niseq package to "Related software" (mne-tools#11909) Minor fixes for ERDS maps example (mne-tools#11904) FIX: Fix pyvista rendering (mne-tools#11896) BUG: Fix epoch splits naming (mne-tools#11876) ENH: Use section-title for HTML anchors in Report (mne-tools#11890) CI: Deploy [circle deploy] MAINT: Clean up whats_new and doc versions (mne-tools#11888) Refactor test_epochs.py::test_split_saving (2 out of 2) (mne-tools#11884) Cross-figure event passing system (mne-tools#11685) MAINT: Post-release deprecations, updates [circle deploy] (mne-tools#11887) MAINT: Release 1.5.0 (mne-tools#11886) [pre-commit.ci] pre-commit autoupdate (mne-tools#11883) Refactor test_epochs.py::test_split_saving (1 out of 2) (mne-tools#11880) FIX: Missing Saccade information in Eyelink File (mne-tools#11877) Improve drawing of annotations with matplotlib (mne-tools#11855) MAINT: Work around NumPy deprecation (mne-tools#11878)
Freesurfer now has a nice
wmparc.mgzthat doesn't fit theasegnaming conventions. I think maybe just removing this check is easiest.