MRG: Enforce specification of all annotation descriptions in event_id…

… if event_id is passed to write_raw_bids() (mne-tools#1086)

* Write all annotations even if event_id was passed

* Implement new logic

* Update changelog

* Fix tests

* Style

doc/whats_new.rst

@@ -12,7 +12,11 @@ What's new?
 Version 0.11.1 (2022-10-21)
 ---------------------------
 
+Version 0.11.1 is a patch release, all changes are listed below.
+For more complete information on the 0.11 version, see changelog below.
+
 - Speed up :func:`mne_bids.read_raw_bids` when lots of events are present by `Alexandre Gramfort`_ (:gh:`1079`)
+- When writing data via :func:`~mne_bids.write_raw_bids`, it is now possible to specify a custom mapping of :class:`mne.Annotations` descriptions to event codes via the ``event_id`` parameter. Previously, passing this parameter would always require to also pass ``events``, and using a custom event code mapping for annotations was impossible, by `Richard Höchenberger`_ (:gh:`1084`)
 
 .. _changes_0_11:
 
@@ -110,6 +114,7 @@ Detailed list of changes
 🪲 Bug fixes
 ^^^^^^^^^^^^
 
+<<<<<<< HEAD
 - Fix ACPC in ``surface RAS`` instead of ``scanner RAS`` in :ref:`ieeg-example` and add convenience functions :func:`mne_bids.convert_montage_to_ras` and :func:`mne_bids.convert_montage_to_mri` to help, by `Alex Rockhill`_ (:gh:`990`)
 
 - Suppress superfluous warnings about MaxShield in many functions when handling Elekta/Neuromag/MEGIN data, by `Richard Höchenberger`_ (:gh:`1000`)
@@ -135,6 +140,9 @@ Detailed list of changes
 - Whenever :func:`~mne_bids.read_raw_bids` encounters a channel type that currently doesn't translate into an appropriate MNE channel type, the channel type will now be set to ``'misc``. Previously, seemingly arbitrary channel types would be applied, e.g. ``'eeg'`` for GSR and temperature channels, by `Richard Höchenberger`_ (:gh:`1052`)
 
 - Fix the incorrect setting of the fields ``ContinuousHeadLocalization`` and ``HeadCoilFrequency`` for Neuromag MEG recordings, by `Eduard Ort`_ (:gh:`1067`)
+=======
+- When writing data containing :class:`mne.Annotations` **and** passing events to :func:`~mne_bids.write_raw_bids`, previously, annotations whose description did not appear in ``event_id`` were silently dropped. We now raise an exception and request users to specify mappings between descriptions and event codes in this case. It is still possible to omit ``event_id`` if no ``events`` are passed, by `Richard Höchenberger`_ (:gh:`1084`)
+>>>>>>> 46b0a530... MRG: Enforce specification of all annotation descriptions in event_id if event_id is passed to write_raw_bids() (#1086)
 
 
 :doc:`Find out what was new in previous releases <whats_new_previous_releases>`

mne_bids/read.py

@@ -124,8 +124,29 @@ def _read_events(events, event_id, raw, bids_path=None):
     else:
         events = read_events(events).astype(int)
 
+    if raw.annotations:
+        if event_id is None:
+            logger.info(
+                'The provided raw data contains annotations, but you did not '
+                'pass an "event_id" mapping from annotation descriptions to '
+                'event codes. We will generate arbitrary event codes. '
+                'To specify custom event codes, please pass "event_id".'
+            )
+        else:
+            desc_without_id = sorted(
+                set(raw.annotations.description) - set(event_id.keys())
+            )
+            if desc_without_id:
+                raise ValueError(
+                    f'The provided raw data contains annotations, but '
+                    f'"event_id" does not contain entries for all annotation '
+                    f'descriptions. The following entries are missing: '
+                    f'{", ".join(desc_without_id)}'
+                )
+
+    # If we have events, convert them to Annotations so they can be easily
+    # merged with existing Annotations.
     if events.size > 0:
-        # Only keep events for which we have an ID <> description mapping.
         ids_without_desc = set(events[:, 2]) - set(event_id.values())
         if ids_without_desc:
             raise ValueError(
@@ -134,9 +155,6 @@ def _read_events(events, event_id, raw, bids_path=None):
                 f'Please add them to the event_id dictionary, or drop them '
                 f'from the events array.'
             )
-        del ids_without_desc
-        mask = [e in list(event_id.values()) for e in events[:, 2]]
-        events = events[mask]
 
         # Append events to raw.annotations. All event onsets are relative to
         # measurement beginning.

mne_bids/tests/test_write.py

@@ -1272,17 +1272,10 @@ def test_eegieeg(dir_name, fname, reader, _bids_validate, tmp_path):
                           match='Encountered data in "double" format'):
             bids_output_path = write_raw_bids(**kwargs)
 
-    event_id = {'Auditory/Left': 1, 'Auditory/Right': 2, 'Visual/Left': 3,
-                'Visual/Right': 4, 'Smiley': 5, 'Button': 32}
-
     with pytest.raises(ValueError,
                        match='You passed events, but no event_id '):
         write_raw_bids(raw, bids_path, events=events)
 
-    with pytest.raises(ValueError,
-                       match='You passed event_id, but no events'):
-        write_raw_bids(raw, bids_path, event_id=event_id)
-
     # check events.tsv is written
     events_tsv_fname = bids_output_path.copy().update(suffix='events',
                                                       extension='.tsv')
@@ -2704,14 +2697,89 @@ def test_annotations(_bids_validate, bad_segments, tmp_path):
     _bids_validate(bids_root)
 
 
+@pytest.mark.parametrize(
+    'write_events', [True, False]  # whether to pass "events" to write_raw_bids
+)
+@pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
+@testing.requires_testing_data
+def test_annotations_and_events(_bids_validate, tmp_path, write_events):
+    """Test combined writing of Annotations and events."""
+    bids_root = tmp_path / 'bids'
+    bids_path = _bids_path.copy().update(root=bids_root, datatype='meg')
+    raw_fname = data_path / 'MEG' / 'sample' / 'sample_audvis_trunc_raw.fif'
+    events_fname = (
+        data_path / 'MEG' / 'sample' / 'sample_audvis_trunc_raw-eve.fif'
+    )
+    events_tsv_fname = bids_path.copy().update(
+        suffix='events',
+        extension='.tsv',
+    )
+
+    events = mne.read_events(events_fname)
+    events = events[events[:, 2] != 0]  # drop unknown "0" events
+    event_id = {'Auditory/Left': 1, 'Auditory/Right': 2, 'Visual/Left': 3,
+                'Visual/Right': 4, 'Smiley': 5, 'Button': 32}
+    raw = _read_raw_fif(raw_fname)
+    annotations = mne.Annotations(
+        # Try to avoid rounding errors.
+        onset=(
+            1 / raw.info['sfreq'] * 600,
+            1 / raw.info['sfreq'] * 600,  # intentional
+            1 / raw.info['sfreq'] * 3000
+        ),
+        duration=(
+            1 / raw.info['sfreq'],
+            1 / raw.info['sfreq'],
+            1 / raw.info['sfreq'] * 200
+        ),
+        description=('BAD_segment', 'EDGE_segment', 'custom'),
+    )
+    raw.set_annotations(annotations)
+
+    # Write annotations while passing event_id
+    # Should raise since annotations descriptions are missing from event_id
+    with pytest.raises(ValueError, match='The following entries are missing'):
+        write_raw_bids(
+            raw,
+            bids_path=bids_path,
+            event_id=event_id,
+            events=events if write_events else None,
+        )
+
+    # Passing a complete mapping should work
+    event_id_with_annots = event_id.copy()
+    event_id_with_annots.update({
+        'BAD_segment': 9999,
+        'EDGE_segment': 10000,
+        'custom': 2000
+    })
+    write_raw_bids(
+        raw,
+        bids_path=bids_path,
+        event_id=event_id_with_annots,
+        events=events if write_events else None,
+    )
+    _bids_validate(bids_root)
+
+    # Ensure all events + annotations were written
+    events_tsv = _from_tsv(events_tsv_fname)
+
+    if write_events:
+        n_events_expected = len(events) + len(raw.annotations)
+    else:
+        n_events_expected = len(raw.annotations)
+
+    assert len(events_tsv['trial_type']) == n_events_expected
+
+
 @pytest.mark.parametrize(
     'drop_undescribed_events',
     [True, False]
 )
 @pytest.mark.filterwarnings(warning_str['channel_unit_changed'])
 @testing.requires_testing_data
 def test_undescribed_events(_bids_validate, drop_undescribed_events, tmp_path):
-    """Test we're behaving correctly if event descriptions are missing."""
+    """Test we're raising if event descriptions are missing."""
     bids_root = tmp_path / 'bids1'
     bids_path = _bids_path.copy().update(root=bids_root, datatype='meg')
     raw_fname = op.join(data_path, 'MEG', 'sample',

mne_bids/write.py

@@ -1337,14 +1337,16 @@ def write_raw_bids(
         If an array, the MNE events array (shape: ``(n_events, 3)``).
         If a path or an array and ``raw.annotations`` exist, the union of
         ``events`` and ``raw.annotations`` will be written.
-        Corresponding descriptions for all event codes (listed in the third
+        Mappings from event names to event codes (listed in the third
         column of the MNE events array) must be specified via the ``event_id``
-        parameter; otherwise, an exception is raised.
+        parameter; otherwise, an exception is raised. If
+        :class:`~mne.Annotations` are present, their descriptions must be
+        included in ``event_id`` as well.
         If ``None``, events will only be inferred from the raw object's
         :class:`~mne.Annotations`.
 
         .. note::
-           If ``not None``, writes the union of ``events`` and
+           If specified, writes the union of ``events`` and
            ``raw.annotations``. If you wish to **only** write
            ``raw.annotations``, pass ``events=None``. If you want to
            **exclude** the events in ``raw.annotations`` from being written,
@@ -1359,7 +1361,10 @@ def write_raw_bids(
         ``events``. The descriptions will be written to the ``trial_type``
         column in ``*_events.tsv``. The dictionary keys correspond to the event
         description,s and the values to the event codes. You must specify a
-        description for all event codes appearing in ``events``.
+        description for all event codes appearing in ``events``. If your data
+        contains :class:`~mne.Annotations`, you can use this parameter to
+        assign event codes to each unique annotation description (mapping from
+        description to event code).
     anonymize : dict | None
         If `None` (default), no anonymization is performed.
         If a dictionary, data will be anonymized depending on the dictionary
@@ -1575,11 +1580,7 @@ def write_raw_bids(
 
     if events is not None and event_id is None:
         raise ValueError('You passed events, but no event_id '
-                         'dictionary. You need to pass both, or neither.')
-
-    if event_id is not None and events is None:
-        raise ValueError('You passed event_id, but no events. '
-                         'You need to pass both, or neither.')
+                         'dictionary.')
 
     _validate_type(item=empty_room, item_name='empty_room',
                    types=(mne.io.BaseRaw, BIDSPath, None))

test_requirements.txt

@@ -5,7 +5,7 @@ matplotlib>=3.1
 pandas>=0.24.0
 nibabel>=2.5
 pybv>=0.7.3
-git+https://github.com/larsoner/openneuro-py@iterate#egg=openneuro-py
+openneuro-py>=2022.1
 EDFlib-Python>=1.0.6
 pymatreader>=0.0.29
 pytest