Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix doc and logging mark_channels #1293

Merged
merged 6 commits into from
Aug 19, 2024
Merged

fix doc and logging mark_channels #1293

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c0b66a2
fix doc and logging mark_channels
sappelhoff Aug 16, 2024
5c04279
sane behavior, add changelog
sappelhoff Aug 16, 2024
4925bb8
Update doc/whats_new.rst
sappelhoff Aug 16, 2024
f345518
Update mne_bids/write.py
sappelhoff Aug 16, 2024
4f85fe8
Update mne_bids/write.py
sappelhoff Aug 16, 2024
8f77101
fix set logic
sappelhoff Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions mne_bids/write.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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``.
Expand Down Expand Up @@ -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):
sappelhoff marked this conversation as resolved.
Show resolved Hide resolved
raise ValueError(
'Setting the status of a channel must only be "good", or "bad".'
)
Expand All @@ -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"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no idea why this was just bad before 🤔 especially because the column is initialized with all good if it wasn't present before.

    # 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.')
        tsv_data["status"] = ["good"] * len(tsv_data["name"])

I think this is also why in the docstring it said something about "default is 'bad'". If we start writing the status for a few channels only, the status for all other channels gets automatically set. However it gets set to "good", not "bad", ... which I find reasonable, but which was misleading in the function docstr.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I followed up on this in 5c04279

f" description: {description}"
)
tsv_data["status"][idx] = status_
Expand Down