From cb32a986c2e64919a1d51ef3f5c39ee1b8f4da1a Mon Sep 17 00:00:00 2001 From: Daniel McCloy Date: Thu, 19 Sep 2024 15:47:03 -0500 Subject: [PATCH 1/4] deprecate [] as indicating all channels --- mne_bids/tests/test_write.py | 5 +++-- mne_bids/write.py | 29 +++++++++++++++++++++++------ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/mne_bids/tests/test_write.py b/mne_bids/tests/test_write.py index c9c2a18eb..ffbb7de3b 100644 --- a/mne_bids/tests/test_write.py +++ b/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"]) diff --git a/mne_bids/write.py b/mne_bids/write.py index 3399f21a9..de58d2809 100644 --- a/mne_bids/write.py +++ b/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 newer, an empty list would mark *all* channels. In version 0.17 and + later, 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): From 1406a2cbdacfac285695f93a0d9a75cd658d0b0b Mon Sep 17 00:00:00 2001 From: Daniel McCloy Date: Thu, 19 Sep 2024 15:52:23 -0500 Subject: [PATCH 2/4] changelog --- doc/whats_new.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/whats_new.rst b/doc/whats_new.rst index 9ae150b71..61da99909 100644 --- a/doc/whats_new.rst +++ b/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=[]) ` 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 ^^^^^^^^^^^^^^^ From f9be73c78bddacabf5b4dd9254a74a1156503c35 Mon Sep 17 00:00:00 2001 From: Daniel McCloy Date: Thu, 19 Sep 2024 15:53:15 -0500 Subject: [PATCH 3/4] typo --- doc/whats_new.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whats_new.rst b/doc/whats_new.rst index 61da99909..42a42a725 100644 --- a/doc/whats_new.rst +++ b/doc/whats_new.rst @@ -43,7 +43,7 @@ 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=[]) ` 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`) +- :func:`mark_channels(..., ch_names=[]) ` 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 From 6faf76485d72caad62e4b61dddcc8f167563fb2b Mon Sep 17 00:00:00 2001 From: Daniel McCloy Date: Thu, 19 Sep 2024 15:55:00 -0500 Subject: [PATCH 4/4] another typo --- mne_bids/write.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mne_bids/write.py b/mne_bids/write.py index de58d2809..b7cf34d02 100644 --- a/mne_bids/write.py +++ b/mne_bids/write.py @@ -2527,8 +2527,8 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non .. versionchanged:: 0.16 The behavior of passing an empty list will change in version 0.17. In version - 0.16 and newer, an empty list would mark *all* channels. In version 0.17 and - later, an empty list will be a no-op (no channels will be marked/changed). + 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