Repository: mne-tools/mne-python

Issue:
ENH: raw.info['bads'] += unchecked
We currently check `raw.info['bads'] = ...` because we can do it in `Info.__setitem__`, but we don't check `raw.info['bads'] += ` because this is a `+=` operation on the bads list. One hopefully easy way to fix this would be to subclass `List` to check any inplace ops (append, extend, +=, ...?) to check if the channels can be added. This would require keeping a reference to the `Info` object, though (because you need the channel names), but that should be possible in principle I think.

Discussion:

Patch:
diff --git a/doc/changes/devel.rst b/doc/changes/devel.rst
index aac780108..b9ec5e577 100644
--- a/doc/changes/devel.rst
+++ b/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`_)
diff --git a/mne/_fiff/meas_info.py b/mne/_fiff/meas_info.py
index 672f805c1..5f3fa7c00 100644
--- a/mne/_fiff/meas_info.py
+++ b/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 @@ class Info(dict, SetChannelsMixin, MontageMixin, ContainsMixin):
         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 @@ class Info(dict, SetChannelsMixin, MontageMixin, ContainsMixin):
                 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 @@ class Info(dict, SetChannelsMixin, MontageMixin, ContainsMixin):
 
     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:
diff --git a/mne/channels/channels.py b/mne/channels/channels.py
index b6c82f27b..d57610d25 100644
--- a/mne/channels/channels.py
+++ b/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()
 
 
diff --git a/mne/forward/forward.py b/mne/forward/forward.py
index 07ea99d59..31aa3c2bd 100644
--- a/mne/forward/forward.py
+++ b/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)
diff --git a/mne/io/artemis123/artemis123.py b/mne/io/artemis123/artemis123.py
index 34a0c0118..8d937067a 100644
--- a/mne/io/artemis123/artemis123.py
+++ b/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
 
 
diff --git a/mne/io/cnt/cnt.py b/mne/io/cnt/cnt.py
index 7d810a0cd..1a6aa15b7 100644
--- a/mne/io/cnt/cnt.py
+++ b/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
 
 
diff --git a/mne/preprocessing/__init__.py b/mne/preprocessing/__init__.py
index 8358f006e..372df010e 100644
--- a/mne/preprocessing/__init__.py
+++ b/mne/preprocessing/__init__.py
@@ -52,7 +52,7 @@ __getattr__, __dir__, __all__ = lazy.attach(
             "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"],
diff --git a/mne/preprocessing/annotate_nan.py b/mne/preprocessing/_annotate_nan.py
similarity index 100%
rename from mne/preprocessing/annotate_nan.py
rename to mne/preprocessing/_annotate_nan.py
diff --git a/mne/preprocessing/ica.py b/mne/preprocessing/ica.py
index 1414f2a4c..15c1d286d 100644
--- a/mne/preprocessing/ica.py
+++ b/mne/preprocessing/ica.py
@@ -1372,8 +1372,8 @@ class ICA(ContainsMixin):
             ]
         with info._unlock(update_redundant=True, check_after=True):
             info["chs"] = ch_info
-            info["bads"] = [ch_names[k] for k in self.exclude]
             info["projs"] = []  # make sure projections are removed.
+        info["bads"] = [ch_names[k] for k in self.exclude]
 
     @verbose
     def score_sources(
diff --git a/mne/viz/topomap.py b/mne/viz/topomap.py
index ec518ec37..bac42416a 100644
--- a/mne/viz/topomap.py
+++ b/mne/viz/topomap.py
@@ -121,11 +121,10 @@ def _prepare_topomap_plot(inst, ch_type, sphere=None):
     clean_ch_names = _clean_names(info["ch_names"])
     for ii, this_ch in enumerate(info["chs"]):
         this_ch["ch_name"] = clean_ch_names[ii]
-    info["bads"] = _clean_names(info["bads"])
     for comp in info["comps"]:
         comp["data"]["col_names"] = _clean_names(comp["data"]["col_names"])
-
     info._update_redundant()
+    info["bads"] = _clean_names(info["bads"])
     info._check_consistency()
 
     # special case for merging grad channels
diff --git a/tutorials/io/60_ctf_bst_auditory.py b/tutorials/io/60_ctf_bst_auditory.py
index 7cb970bc9..01a65ef32 100644
--- a/tutorials/io/60_ctf_bst_auditory.py
+++ b/tutorials/io/60_ctf_bst_auditory.py
@@ -153,7 +153,7 @@ del saccade_epochs, saccades_events, projs_eog, projs_saccade  # To save memory
 # plotted by adding the event list as a keyword argument. As the bad segments
 # and saccades were added as annotations to the raw data, they are plotted as
 # well.
-raw.plot(block=True)
+raw.plot()
 
 # %%
 # Typical preprocessing step is the removal of power line artifact (50 Hz or


Test patch:
diff --git a/mne/_fiff/tests/test_meas_info.py b/mne/_fiff/tests/test_meas_info.py
index 844d04fc6..feb30400d 100644
--- a/mne/_fiff/tests/test_meas_info.py
+++ b/mne/_fiff/tests/test_meas_info.py
@@ -59,6 +59,7 @@ from mne._fiff.meas_info import (
     _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():
diff --git a/mne/_fiff/tests/test_pick.py b/mne/_fiff/tests/test_pick.py
index 51aaa6b36..786a14a72 100644
--- a/mne/_fiff/tests/test_pick.py
+++ b/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])