BUG: Fix bug with validation of info["bads"] (mne-tools#12039)

Author: larsoner

doc/changes/devel.rst

@@ -55,6 +55,7 @@ Bugs
 - Fix bug with :meth:`~mne.viz.Brain.add_annotation` when reading an annotation from a file with both hemispheres shown (:gh:`11946` by `Marijn van Vliet`_)
 - Fix bug with axis clip box boundaries in :func:`mne.viz.plot_evoked_topo` and related functions (:gh:`11999` by `Eric Larson`_)
 - Fix bug with ``subject_info`` when loading data from and exporting to EDF file (:gh:`11952` by `Paul Roujansky`_)
+- Fix bug with delayed checking of :class:`info["bads"] <mne.Info>` (:gh:`12038` by `Eric Larson`_)
 - Fix handling of channel information in annotations when loading data from and exporting to EDF file (:gh:`11960` :gh:`12017` by `Paul Roujansky`_)
 - Add missing ``overwrite`` and ``verbose`` parameters to :meth:`Transform.save() <mne.transforms.Transform.save>` (:gh:`12004` by `Marijn van Vliet`_)
 - Correctly prune channel-specific :class:`~mne.Annotations` when creating :class:`~mne.Epochs` without the channel(s) included in the channel specific annotations (:gh:`12010` by `Mathieu Scheltienne`_)

mne/_fiff/meas_info.py

@@ -939,18 +939,52 @@ def _check_ch_keys(ch, ci, name='info["chs"]', check_min=True):
             )
 
 
-# As options are added here, test_meas_info.py:test_info_bad should be updated
-def _check_bads(bads):
+def _check_bads_info_compat(bads, info):
     _validate_type(bads, list, "bads")
-    return bads
+    if not len(bads):
+        return  # e.g. in empty_info
+    for bi, bad in enumerate(bads):
+        _validate_type(bad, str, f"bads[{bi}]")
+    if "ch_names" not in info:  # somewhere in init, or deepcopy, or _empty_info, etc.
+        return
+    missing = [bad for bad in bads if bad not in info["ch_names"]]
+    if len(missing) > 0:
+        raise ValueError(f"bad channel(s) {missing} marked do not exist in info")
+
+
+class MNEBadsList(list):
+    """Subclass of bads that checks inplace operations."""
+
+    def __init__(self, *, bads, info):
+        _check_bads_info_compat(bads, info)
+        self._mne_info = info
+        super().__init__(bads)
+
+    def extend(self, iterable):
+        if not isinstance(iterable, list):
+            iterable = list(iterable)
+        _check_bads_info_compat(iterable, self._mne_info)
+        return super().extend(iterable)
+
+    def append(self, x):
+        return self.extend([x])
 
+    def __iadd__(self, x):
+        self.extend(x)
+        return self
+
+
+# As options are added here, test_meas_info.py:test_info_bad should be updated
+def _check_bads(bads, *, info):
+    return MNEBadsList(bads=bads, info=info)
 
-def _check_description(description):
+
+def _check_description(description, *, info):
     _validate_type(description, (None, str), "info['description']")
     return description
 
 
-def _check_dev_head_t(dev_head_t):
+def _check_dev_head_t(dev_head_t, *, info):
     from ..transforms import Transform, _ensure_trans
 
     _validate_type(dev_head_t, (Transform, None), "info['dev_head_t']")
@@ -959,23 +993,23 @@ def _check_dev_head_t(dev_head_t):
     return dev_head_t
 
 
-def _check_experimenter(experimenter):
+def _check_experimenter(experimenter, *, info):
     _validate_type(experimenter, (None, str), "experimenter")
     return experimenter
 
 
-def _check_line_freq(line_freq):
+def _check_line_freq(line_freq, *, info):
     _validate_type(line_freq, (None, "numeric"), "line_freq")
     line_freq = float(line_freq) if line_freq is not None else line_freq
     return line_freq
 
 
-def _check_subject_info(subject_info):
+def _check_subject_info(subject_info, *, info):
     _validate_type(subject_info, (None, dict), "subject_info")
     return subject_info
 
 
-def _check_device_info(device_info):
+def _check_device_info(device_info, *, info):
     _validate_type(
         device_info,
         (
@@ -987,7 +1021,7 @@ def _check_device_info(device_info):
     return device_info
 
 
-def _check_helium_info(helium_info):
+def _check_helium_info(helium_info, *, info):
     _validate_type(
         helium_info,
         (
@@ -1472,7 +1506,7 @@ class Info(dict, SetChannelsMixin, MontageMixin, ContainsMixin):
         "sfreq": "sfreq cannot be set directly. "
         "Please use method inst.resample() instead.",
         "subject_info": _check_subject_info,
-        "temp": lambda x: x,
+        "temp": lambda x, info=None: x,
         "utc_offset": "utc_offset cannot be set directly.",
         "working_dir": "working_dir cannot be set directly.",
         "xplotter_layout": "xplotter_layout cannot be set directly.",
@@ -1482,6 +1516,8 @@ def __init__(self, *args, **kwargs):
         self._unlocked = True
         super().__init__(*args, **kwargs)
         # Deal with h5io writing things as dict
+        if "bads" in self:
+            self["bads"] = MNEBadsList(bads=self["bads"], info=self)
         for key in ("dev_head_t", "ctf_head_t", "dev_ctf_t"):
             _format_trans(self, key)
         for res in self.get("hpi_results", []):
@@ -1526,7 +1562,9 @@ def __setitem__(self, key, val):
                 if not unlocked:
                     raise RuntimeError(self._attributes[key])
             else:
-                val = self._attributes[key](val)  # attribute checker function
+                val = self._attributes[key](
+                    val, info=self
+                )  # attribute checker function
         else:
             raise RuntimeError(
                 f"Info does not support directly setting the key {repr(key)}. "
@@ -1724,16 +1762,6 @@ def __deepcopy__(self, memodict):
 
     def _check_consistency(self, prepend_error=""):
         """Do some self-consistency checks and datatype tweaks."""
-        missing = [bad for bad in self["bads"] if bad not in self["ch_names"]]
-        if len(missing) > 0:
-            msg = "%sbad channel(s) %s marked do not exist in info"
-            raise RuntimeError(
-                msg
-                % (
-                    prepend_error,
-                    missing,
-                )
-            )
         meas_date = self.get("meas_date")
         if meas_date is not None:
             if (
@@ -3335,7 +3363,7 @@ def _force_update_info(info_base, info_target):
         The Info object(s) you wish to overwrite using info_base. These objects
         will be modified in-place.
     """
-    exclude_keys = ["chs", "ch_names", "nchan"]
+    exclude_keys = ["chs", "ch_names", "nchan", "bads"]
     info_target = np.atleast_1d(info_target).ravel()
     all_infos = np.hstack([info_base, info_target])
     for ii in all_infos:

mne/_fiff/tests/test_meas_info.py

@@ -59,6 +59,7 @@
     _dt_to_stamp,
     _add_timedelta_to_stamp,
     _read_extended_ch_info,
+    MNEBadsList,
 )
 from mne.minimum_norm import (
     make_inverse_operator,
@@ -495,8 +496,8 @@ def test_check_consistency():
 
     # Bad channels that are not in the info object
     info2 = info.copy()
-    info2["bads"] = ["b", "foo", "bar"]
-    pytest.raises(RuntimeError, info2._check_consistency)
+    with pytest.raises(ValueError, match="do not exist"):
+        info2["bads"] = ["b", "foo", "bar"]
 
     # Bad data types
     info2 = info.copy()
@@ -1088,21 +1089,42 @@ def test_pickle(fname_info, unlocked):
 
 def test_info_bad():
     """Test our info sanity checkers."""
-    info = create_info(2, 1000.0, "eeg")
+    info = create_info(5, 1000.0, "eeg")
     info["description"] = "foo"
     info["experimenter"] = "bar"
     info["line_freq"] = 50.0
     info["bads"] = info["ch_names"][:1]
     info["temp"] = ("whatever", 1.0)
-    # After 0.24 these should be pytest.raises calls
-    check, klass = pytest.raises, RuntimeError
-    with check(klass, match=r"info\['temp'\]"):
+    with pytest.raises(RuntimeError, match=r"info\['temp'\]"):
         info["bad_key"] = 1.0
     for key, match in [("sfreq", r"inst\.resample"), ("chs", r"inst\.add_channels")]:
-        with check(klass, match=match):
+        with pytest.raises(RuntimeError, match=match):
             info[key] = info[key]
     with pytest.raises(ValueError, match="between meg<->head"):
         info["dev_head_t"] = Transform("mri", "head", np.eye(4))
+    assert isinstance(info["bads"], MNEBadsList)
+    with pytest.raises(ValueError, match="do not exist in info"):
+        info["bads"] = ["foo"]
+    assert isinstance(info["bads"], MNEBadsList)
+    with pytest.raises(ValueError, match="do not exist in info"):
+        info["bads"] += ["foo"]
+    assert isinstance(info["bads"], MNEBadsList)
+    with pytest.raises(ValueError, match="do not exist in info"):
+        info["bads"].append("foo")
+    assert isinstance(info["bads"], MNEBadsList)
+    with pytest.raises(ValueError, match="do not exist in info"):
+        info["bads"].extend(["foo"])
+    assert isinstance(info["bads"], MNEBadsList)
+    x = info["bads"]
+    with pytest.raises(ValueError, match="do not exist in info"):
+        x.append("foo")
+    assert info["bads"] == info["ch_names"][:1]  # unchonged
+    x = info["bads"] + info["ch_names"][1:2]
+    assert x == info["ch_names"][:2]
+    assert not isinstance(x, MNEBadsList)  # plain list
+    x = info["ch_names"][1:2] + info["bads"]
+    assert x == info["ch_names"][1::-1]  # like [1, 0] in fancy indexing
+    assert not isinstance(x, MNEBadsList)  # plain list
 
 
 def test_get_montage():

mne/_fiff/tests/test_pick.py

@@ -567,8 +567,8 @@ def test_clean_info_bads():
 
     info = pick_info(raw.info, picks_meg)
     info._check_consistency()
-    info["bads"] += ["EEG 053"]
-    pytest.raises(RuntimeError, info._check_consistency)
+    with pytest.raises(ValueError, match="do not exist"):
+        info["bads"] += ["EEG 053"]
     with pytest.raises(ValueError, match="unique"):
         pick_info(raw.info, [0, 0])
 

mne/channels/channels.py

@@ -976,7 +976,7 @@ def rename_channels(info, mapping, allow_duplicates=False, *, verbose=None):
         raise ValueError("New channel names are not unique, renaming failed")
 
     # do the remapping in info
-    info["bads"] = bads
+    info["bads"] = []
     ch_names_mapping = dict()
     for ch, ch_name in zip(info["chs"], ch_names):
         ch_names_mapping[ch["ch_name"]] = ch_name
@@ -989,6 +989,7 @@ def rename_channels(info, mapping, allow_duplicates=False, *, verbose=None):
                 proj["data"]["col_names"], ch_names_mapping
             )
     info._update_redundant()
+    info["bads"] = bads
     info._check_consistency()
 
 

mne/forward/forward.py

@@ -458,11 +458,9 @@ def _read_forward_meas_info(tree, fid):
     else:
         raise ValueError("MEG/head coordinate transformation not found")
 
-    info["bads"] = _read_bad_channels(
-        fid, parent_meg, ch_names_mapping=ch_names_mapping
-    )
+    bads = _read_bad_channels(fid, parent_meg, ch_names_mapping=ch_names_mapping)
     # clean up our bad list, old versions could have non-existent bads
-    info["bads"] = [bad for bad in info["bads"] if bad in info["ch_names"]]
+    info["bads"] = [bad for bad in bads if bad in info["ch_names"]]
 
     # Check if a custom reference has been applied
     tag = find_tag(fid, parent_mri, FIFF.FIFF_MNE_CUSTOM_REF)

mne/io/artemis123/artemis123.py

@@ -192,7 +192,7 @@ def _get_artemis123_info(fname, pos_fname=None):
     # load mne loc dictionary
     loc_dict = _load_mne_locs()
     info["chs"] = []
-    info["bads"] = []
+    bads = []
 
     for i, chan in enumerate(header_info["channels"]):
         # build chs struct
@@ -209,7 +209,7 @@ def _get_artemis123_info(fname, pos_fname=None):
         # a value of another ref channel to make writers/readers happy.
         if t["cal"] == 0:
             t["cal"] = 4.716e-10
-            info["bads"].append(t["ch_name"])
+            bads.append(t["ch_name"])
         t["loc"] = loc_dict.get(chan["name"], np.zeros(12))
 
         if chan["name"].startswith("MEG"):
@@ -247,7 +247,7 @@ def _get_artemis123_info(fname, pos_fname=None):
             t["coil_type"] = FIFF.FIFFV_COIL_NONE
             t["kind"] = FIFF.FIFFV_MISC_CH
             t["unit"] = FIFF.FIFF_UNIT_V
-            info["bads"].append(t["ch_name"])
+            bads.append(t["ch_name"])
 
         elif chan["name"].startswith(("AUX", "TRG", "MIO")):
             t["coil_type"] = FIFF.FIFFV_COIL_NONE
@@ -268,10 +268,7 @@ def _get_artemis123_info(fname, pos_fname=None):
         # append this channel to the info
         info["chs"].append(t)
         if chan["FLL_ResetLock"] == "TRUE":
-            info["bads"].append(t["ch_name"])
-
-    # reduce info['bads'] to unique set
-    info["bads"] = list(set(info["bads"]))
+            bads.append(t["ch_name"])
 
     # HPI information
     # print header_info.keys()
@@ -313,6 +310,9 @@ def _get_artemis123_info(fname, pos_fname=None):
 
     info._unlocked = False
     info._update_redundant()
+    # reduce info['bads'] to unique set
+    info["bads"] = list(set(bads))
+    del bads
     return info, header_info
 
 

mne/io/cnt/cnt.py

@@ -404,12 +404,12 @@ def _get_cnt_info(input_fname, eog, ecg, emg, misc, data_format, date_format):
         meas_date=meas_date,
         dig=dig,
         description=session_label,
-        bads=bads,
         subject_info=subject_info,
         chs=chs,
     )
     info._unlocked = False
     info._update_redundant()
+    info["bads"] = bads
     return info, cnt_info
 
 

mne/preprocessing/__init__.py

@@ -52,7 +52,7 @@
             "read_fine_calibration",
             "write_fine_calibration",
         ],
-        "annotate_nan": ["annotate_nan"],
+        "_annotate_nan": ["annotate_nan"],
         "interpolate": ["equalize_bads", "interpolate_bridged_electrodes"],
         "_css": ["cortical_signal_suppression"],
         "hfc": ["compute_proj_hfc"],