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.

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

[MRG] auto-add event_id key for BAD_ACQ_SKIP #1258

Merged
merged 5 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
3 changes: 3 additions & 0 deletions doc/whats_new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ The following authors had contributed before. Thank you for sticking around!
* `Laetitia Fesselier`_
* `Richard Höchenberger`_
* `Stefan Appelhoff`_
* `Daniel McCloy`_

Detailed list of changes
~~~~~~~~~~~~~~~~~~~~~~~~
Expand All @@ -46,6 +47,8 @@ Detailed list of changes
For example, If ``run=1`` is passed to MNE-BIDS, it will no longer be silently auto-converted to ``run-01``, by `Alex Rockhill`_ (:gh:`1215`)
- MNE-BIDS will no longer warn about missing leading punctuation marks for extensions passed :class:`~mne_bids.BIDSPath`.
For example, MNE-BIDS will now silently auto-convert ``edf`` to ```.edf``, by `Alex Rockhill`_ (:gh:`1215`)
- MNE-BIDS will no longer error during `~mne_bids.write_raw_bids` if there are ``BAD_ACQ_SKIP`` annotations in the raw file and no corresponding key in ``event_id``.
Instead, it will automatically add the necessary key and assign an unused integer event code to it. By `Daniel McCloy`_ (:gh:`1258`)

🛠 Requirements
^^^^^^^^^^^^^^^
Expand Down
11 changes: 11 additions & 0 deletions mne_bids/read.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,20 @@ def _read_events(events, event_id, raw, bids_path=None):
'To specify custom event codes, please pass "event_id".'
)
else:
special_annots = {"BAD_ACQ_SKIP"}
desc_without_id = sorted(
set(raw.annotations.description) - set(event_id.keys())
)
# auto-add entries to `event_id` for "special" annotation values
# (but only if they're needed)
if set(desc_without_id) & special_annots:
existing_ids = set(event_id.values())
for annot in special_annots:
# don't change value if key is present; use a value guaranteed to
# not be in use
event_id.setdefault(annot, max(existing_ids) + 1)
drammock marked this conversation as resolved.
Show resolved Hide resolved
# remove the "special" annots from the list of problematic annots
desc_without_id = sorted(set(desc_without_id) - special_annots)
if desc_without_id:
raise ValueError(
f"The provided raw data contains annotations, but "
Expand Down
41 changes: 31 additions & 10 deletions mne_bids/tests/test_read.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@
acq = "01"
task = "testing"

sample_data_event_id = {
"Auditory/Left": 1,
"Auditory/Right": 2,
"Visual/Left": 3,
"Visual/Right": 4,
"Smiley": 5,
"Button": 32,
}

_bids_path = BIDSPath(
subject=subject_id, session=session_id, run=run, acquisition=acq, task=task
)
Expand Down Expand Up @@ -214,14 +223,6 @@ def test_get_head_mri_trans(tmp_path):
"""Test getting a trans object from BIDS data."""
nib = pytest.importorskip("nibabel")

event_id = {
"Auditory/Left": 1,
"Auditory/Right": 2,
"Visual/Left": 3,
"Visual/Right": 4,
"Smiley": 5,
"Button": 32,
}
events_fname = op.join(
data_path, "MEG", "sample", "sample_audvis_trunc_raw-eve.fif"
)
Expand All @@ -234,7 +235,9 @@ def test_get_head_mri_trans(tmp_path):
# Write it to BIDS
raw = _read_raw_fif(raw_fname)
bids_path = _bids_path.copy().update(root=tmp_path, datatype="meg", suffix="meg")
write_raw_bids(raw, bids_path, events=events, event_id=event_id, overwrite=False)
write_raw_bids(
raw, bids_path, events=events, event_id=sample_data_event_id, overwrite=False
)

# We cannot recover trans if no MRI has yet been written
with pytest.raises(FileNotFoundError, match="Did not find"):
Expand Down Expand Up @@ -300,7 +303,7 @@ def test_get_head_mri_trans(tmp_path):
# sidecar, and also accept "nasion" instead of just "NAS"
raw = _read_raw_fif(raw_fname)
write_raw_bids(
raw, bids_path, events=events, event_id=event_id, overwrite=True
raw, bids_path, events=events, event_id=sample_data_event_id, overwrite=True
) # overwrite with new acq
t1w_bids_path = write_anat(
t1w_mgh, bids_path=t1w_bids_path, landmarks=landmarks, overwrite=True
Expand Down Expand Up @@ -578,6 +581,24 @@ def test_keep_essential_annotations(tmp_path):
assert raw_read.annotations[0]["description"] == raw.annotations[0]["description"]


@testing.requires_testing_data
def test_adding_essential_annotations_to_dict(tmp_path):
"""Test that essential Annotations are auto-added to the `event_id` dictionary."""
raw = _read_raw_fif(raw_fname)
annotations = mne.Annotations(
onset=[raw.times[0]], duration=[1], description=["BAD_ACQ_SKIP"]
)
raw.set_annotations(annotations)
events = mne.find_events(raw)

# see that no error is raised for missing event_id key for BAD_ACQ_SKIP
bids_path = BIDSPath(subject="01", task="task", datatype="meg", root=tmp_path)
with pytest.warns(RuntimeWarning, match="Acquisition skips detected"):
write_raw_bids(
raw, bids_path, overwrite=True, events=events, event_id=sample_data_event_id
)


@pytest.mark.filterwarnings(warning_str["channel_unit_changed"])
@testing.requires_testing_data
def test_handle_scans_reading(tmp_path):
Expand Down
Loading