From c0b66a22bd1ec485b81321dd60b1aada08ab13d0 Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Fri, 16 Aug 2024 11:44:09 +0200 Subject: [PATCH 1/6] fix doc and logging mark_channels --- mne_bids/write.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/mne_bids/write.py b/mne_bids/write.py index d34b0dbb4..5bb8026cd 100644 --- a/mne_bids/write.py +++ b/mne_bids/write.py @@ -2463,9 +2463,8 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non The names of the channel(s) to mark with a ``status`` and possibly a ``description``. Can be an empty list to indicate all channel names. status : 'good' | 'bad' | list of str - The status of the channels ('good', or 'bad'). Default is 'bad'. If it - is a list, then must be a list of 'good', or 'bad' that has the same - length as ``ch_names``. + 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``. descriptions : None | str | list of str Descriptions of the reasons that lead to the exclusion of the channel(s). If a list, it must match the length of ``ch_names``. @@ -2543,7 +2542,7 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non f"({len(ch_names)})." ) - if not all(status in ["good", "bad"] for status in status): + if not all(status_ in ["good", "bad"] for status_ in status): raise ValueError( 'Setting the status of a channel must only be "good", or "bad".' ) @@ -2565,7 +2564,7 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non idx = tsv_data["name"].index(ch_name) logger.info( f"Processing channel {ch_name}:\n" - f" status: bad\n" + f" status: {status_}\n" f" description: {description}" ) tsv_data["status"][idx] = status_ From 5c042796f634e96c220682328de7968082c7b874 Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Fri, 16 Aug 2024 12:10:52 +0200 Subject: [PATCH 2/6] sane behavior, add changelog --- doc/whats_new.rst | 1 + mne_bids/write.py | 27 +++++++++++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/doc/whats_new.rst b/doc/whats_new.rst index 4b6d2c846..174e8c36c 100644 --- a/doc/whats_new.rst +++ b/doc/whats_new.rst @@ -39,6 +39,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 created a ``status_description`` column filled with ``n/a`` in the ``channels.tsv`` file, by `Stefan Appelhoff`_ (:gh:`1293`) 🛠 Requirements ^^^^^^^^^^^^^^^ diff --git a/mne_bids/write.py b/mne_bids/write.py index 5bb8026cd..cb4774da7 100644 --- a/mne_bids/write.py +++ b/mne_bids/write.py @@ -2460,17 +2460,24 @@ 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 possibly a + 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. 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``. descriptions : None | str | list of str - Descriptions of the reasons that lead to the exclusion of the + Descriptions of the reasons that lead to the marking ('good' or 'bad') of the channel(s). If a list, it must match the length of ``ch_names``. If ``None``, no descriptions are added. %(verbose)s + Notes + ----- + If the 'status' or 'status_description' columns were not present in the + corresponding tsv file before using this function, they may be created with default + values ('good' for status, 'n/a' for status_description) for all channels that are + not differently specified (by using ``ch_names``, ``status``, and ``descriptions``). + Examples -------- Mark a single channel as bad. @@ -2525,7 +2532,9 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non # set descriptions based on how it's passed in if isinstance(descriptions, str): descriptions = [descriptions] * len(ch_names) + write_descriptions = True elif not descriptions: + write_descriptions = False descriptions = [None] * len(ch_names) # make sure statuses is a list of strings @@ -2549,11 +2558,17 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non # Read sidecar and create required columns if they do not exist. if "status" not in tsv_data: - logger.info('No "status" column found in input file. Creating.') + logger.info( + 'No "status" column found in input file.' + 'Creating it with default value "good".' + ) tsv_data["status"] = ["good"] * len(tsv_data["name"]) - if "status_description" not in tsv_data: - logger.info('No "status_description" column found in input file. Creating.') + if "status_description" not in tsv_data and write_descriptions: + logger.info( + 'No "status_description" column found in input file. ' + 'Creating it with default value "n/a".' + ) tsv_data["status_description"] = ["n/a"] * len(tsv_data["name"]) # Now actually mark the user-requested channels as bad. @@ -2570,7 +2585,7 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non tsv_data["status"][idx] = status_ # only write if the description was passed in - if description is not None: + if description: tsv_data["status_description"][idx] = description _write_tsv(channels_fname, tsv_data, overwrite=True) From 4925bb8a333ebdf5016ca15610750952ce601a83 Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Fri, 16 Aug 2024 20:21:52 +0200 Subject: [PATCH 3/6] Update doc/whats_new.rst Co-authored-by: Daniel McCloy --- 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 174e8c36c..42133bedf 100644 --- a/doc/whats_new.rst +++ b/doc/whats_new.rst @@ -39,7 +39,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 created a ``status_description`` column filled with ``n/a`` in the ``channels.tsv`` file, by `Stefan Appelhoff`_ (:gh:`1293`) +- :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`) 🛠 Requirements ^^^^^^^^^^^^^^^ From f345518a64a82ac1c796d9fb44ade4f0004ecf5c Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Fri, 16 Aug 2024 20:25:42 +0200 Subject: [PATCH 4/6] Update mne_bids/write.py Co-authored-by: Daniel McCloy --- mne_bids/write.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne_bids/write.py b/mne_bids/write.py index cb4774da7..39843140b 100644 --- a/mne_bids/write.py +++ b/mne_bids/write.py @@ -2559,7 +2559,7 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non # Read sidecar and create required columns if they do not exist. if "status" not in tsv_data: logger.info( - 'No "status" column found in input file.' + 'No "status" column found in channels file.' 'Creating it with default value "good".' ) tsv_data["status"] = ["good"] * len(tsv_data["name"]) From 4f85fe811b46526b06b55adaba0e76a5e59c30b7 Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Fri, 16 Aug 2024 20:26:16 +0200 Subject: [PATCH 5/6] Update mne_bids/write.py Co-authored-by: Daniel McCloy --- mne_bids/write.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne_bids/write.py b/mne_bids/write.py index 39843140b..46590d5fb 100644 --- a/mne_bids/write.py +++ b/mne_bids/write.py @@ -2551,7 +2551,7 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non f"({len(ch_names)})." ) - if not all(status_ in ["good", "bad"] for status_ in status): + if set(status) != {"good", "bad"}: raise ValueError( 'Setting the status of a channel must only be "good", or "bad".' ) From 8f77101cf20f693c90b9038967ab74d39e093845 Mon Sep 17 00:00:00 2001 From: Stefan Appelhoff Date: Mon, 19 Aug 2024 10:53:12 +0200 Subject: [PATCH 6/6] fix set logic --- mne_bids/write.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne_bids/write.py b/mne_bids/write.py index 46590d5fb..c7bd1896d 100644 --- a/mne_bids/write.py +++ b/mne_bids/write.py @@ -2551,7 +2551,7 @@ def mark_channels(bids_path, *, ch_names, status, descriptions=None, verbose=Non f"({len(ch_names)})." ) - if set(status) != {"good", "bad"}: + if not set(status).issubset({"good", "bad"}): raise ValueError( 'Setting the status of a channel must only be "good", or "bad".' )