deprecate empty list as indicator for marking all channels (#1307)

doc/whats_new.rst

@@ -43,6 +43,8 @@ Detailed list of changes
 - :func:`mne_bids.read_raw_bids` no longer warns about unit changes in channels upon reading, as that information is taken from ``channels.tsv`` and judged authorative, by `Stefan Appelhoff`_ (:gh:`1282`)
 - MEG OPM channels are now experimentally included, by `Amaia Benitez`_ (:gh:`1222`)
 - :func:`mne_bids.mark_channels` will no longer create a ``status_description`` column filled with ``n/a`` in the ``channels.tsv`` file, by `Stefan Appelhoff`_ (:gh:`1293`)
+- :func:`mark_channels(..., ch_names=[]) <mne_bids.mark_channels>` now raises a deprecation warning, and in future its behavior will change from marking *all* channels to marking *no* channels; to avoid the warning use ``mark_channels(..., ch_names="all")``, by `Daniel McCloy`_ (:gh:`1307`)
+
 
 🛠 Requirements
 ^^^^^^^^^^^^^^^

examples/mark_bad_channels.py

@@ -164,9 +164,9 @@
 
 # %%
 # Lastly, if you're looking for a way to mark all channels as good, simply
-# pass an empty list as ``ch_names``, combined with ``overwrite=True``:
+# pass ``ch_names="all"``, combined with ``overwrite=True``:
 
-bads = []
+bads = "all"
 mark_channels(bids_path=bids_path, ch_names=bads, status="bad", verbose=False)
 
 raw = read_raw_bids(bids_path=bids_path, verbose=False)

mne_bids/commands/mne_bids_mark_channels.py

@@ -98,7 +98,7 @@ def run():
         parser.error("You must specify some --ch_name parameters.")
 
     status = opt.status
-    ch_names = [] if opt.ch_names == [""] else opt.ch_names
+    ch_names = "all" if opt.ch_names == [""] else opt.ch_names
     bids_path = BIDSPath(
         subject=opt.subject,
         session=opt.session,

mne_bids/inspect.py

@@ -446,7 +446,7 @@ def _save_bads(*, bads, descriptions, bids_path):
         The values to be written to the `status_description` column.
     """
     # We first make all channels not passed as bad here to be marked as good.
-    mark_channels(bids_path=bids_path, ch_names=[], status="good")
+    mark_channels(bids_path=bids_path, ch_names="all", status="good")
     mark_channels(
         bids_path=bids_path, ch_names=bads, status="bad", descriptions=descriptions
     )

mne_bids/tests/test_write.py

@@ -2490,7 +2490,7 @@ def test_mark_channels(
     # Mark `existing_ch_names` as bad in raw and sidecar TSV before we
     # begin our actual tests, which should then add additional channels
     # to the list of bads, retaining the ones we're specifying here.
-    mark_channels(ch_names=[], bids_path=bids_path, status="good", verbose=False)
+    mark_channels(ch_names="all", bids_path=bids_path, status="good", verbose=False)
     _bids_validate(bids_root)
     raw = read_raw_bids(bids_path=bids_path, verbose=False)
     # Order is not preserved
@@ -2560,7 +2560,8 @@ def test_mark_channel_roundtrip(tmp_path):
 
     ch_names = raw.ch_names
     # first mark all channels as good
-    mark_channels(bids_path, ch_names=[], status="good", verbose=False)
+    with pytest.warns(FutureWarning, match=r"`mark_channels\(\.\.\., ch_names=\[\]\)`"):
+        mark_channels(bids_path, ch_names=[], status="good", verbose=False)
     tsv_data = _from_tsv(channels_fname)
     assert all(status == "good" for status in tsv_data["status"])
 

mne_bids/write.py

@@ -2522,8 +2522,14 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non
         type (e.g., only EEG or MEG data) is present in the dataset, it will be
         selected automatically.
     ch_names : str | list of str
-        The names of the channel(s) to mark with a ``status`` and optionally a
-        ``description``. Can be an empty list to indicate all channel names.
+        The name(s) of the channel(s) to mark with a ``status`` (and optionally a
+        ``description``). The special value ``"all"`` will mark all channels.
+
+        .. versionchanged:: 0.16
+           The behavior of passing an empty list will change in version 0.17. In version
+           0.16 and older, an empty list would mark *all* channels. In version 0.17 and
+           newer, an empty list will be a no-op (no channels will be marked/changed).
+
     status : 'good' | 'bad' | list of str
         The status of the channels ('good', or 'bad'). If it is a list, then must be a
         list of 'good', or 'bad' that has the same length as ``ch_names``.
@@ -2586,10 +2592,21 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non
 
     # if an empty list is passed in, then these are the entire list
     # of channels
-    if ch_names == []:
-        ch_names = tsv_data["name"]
-    elif isinstance(ch_names, str):
-        ch_names = [ch_names]
+    if list(ch_names) == []:  # casting to list avoids error if ch_names is np.ndarray
+        warn(
+            "In version 0.17, the behavior of `mark_channels(..., ch_names=[])` will "
+            "change, from marking *all* channels to marking *no* channels. Pass "
+            "ch_names='all' instead of ch_names=[] to keep the old behavior and "
+            "avoid this warning.",
+            FutureWarning,
+        )
+        ch_names = "all"
+    # TODO ↑↑↑ remove prior conditional block after 0.16 release ↑↑↑
+    if isinstance(ch_names, str):
+        if ch_names == "all":
+            ch_names = tsv_data["name"]
+        else:
+            ch_names = [ch_names]
 
     # set descriptions based on how it's passed in
     if isinstance(descriptions, str):