We're not writing all annotations if events are passed to write_raw_bids() as well

[hoechenberger]

Description of the problem

This problem came up in https://mne.discourse.group/t/mne-bids-structure-one-subject-is-different/5687/

The user concatenated raws, which creates an edge annotation. However, after writing the data via write_raw_bids(), the annotation didn't show up in events.tsv. Turns out the user passed events and event_id to write_raw_bids() too, which caused this issue to surface.

Steps to reproduce

In the following code, the concatenated raw has an edge and a bad annotation, but they don't end up in the events.tsv sidecar:

# %%
import pandas as pd
import mne
import mne_bids

sample_dir = mne.datasets.sample.data_path()
sample_fname = sample_dir / 'MEG' / 'sample' / 'sample_audvis_raw.fif'

raw = mne.io.read_raw_fif(sample_fname)
raw_concat = mne.concatenate_raws([raw.copy(), raw])
events = mne.find_events(raw_concat)
event_id = {
    'Auditory/Left': 1,
    'Auditory/Right': 2,
    'Visual/Left': 3,
    'Visual/Right': 4,
    'Smiley': 5,
    'Button': 32
}

# %%
bp = mne_bids.BIDSPath(
    subject='sample',
    task='sample',
    suffix='meg',
    extension='.fif',
    datatype='meg',
    root='/tmp/bids-root'
)
mne_bids.write_raw_bids(
    raw_concat,
    bids_path=bp,
    events=events,
    event_id=event_id,
    overwrite=True,
    allow_preload=True,
    format='FIF'
)

# %%
events_tsv_path = (
    bp
    .copy()
    .update(suffix='events', extension='.tsv')
)
events_tsv = pd.read_csv(events_tsv_path, sep='\t')
print(events_tsv['trial_type'].unique())

Expected results

We should see 642 events in events.tsv, including the edge and bad annotations.

Actual results

The edge and bad annotations are missing from the events.tsv sidecar.

Additional information

The problem occurs here:

mne-bids/mne_bids/read.py

Lines 159 to 163 in 9231836

	all_events, all_desc = events_from_annotations(
	raw,
	event_id=event_id,
	regexp=None # Include `BAD_` and `EDGE_` Annotations, too.
	)

While in line 162 we explicitly try to include edge and bad annotations, the documentation of events_from_annotations() also states that any annotations that do not appear in event_id will simply be dropped.

IMHO the cleanest solution here would be to force users to always supply an event_id dictionary. Currently it's optional if no events are passed, which causes us to generate arbitrary event codes for annotations – which at least one user complained about not too long ago. Of course this would break lots of existing code.

We could of course also add special handling for all annotations for which no event_id mapping has been supplied, and we simply generate some new (arbitrary) event codes.

@sappelhoff @agramfort WDYT?

[hoechenberger]

Another option would be to keep the behavior as-is if only annotations are present (and no events and event_id are passed), and we only enforce all annotations to be listed in event_id if events and annoations are passed.

This should break significantly less existing user code!!

[hoechenberger]

Just for clarification, simply not passing events and event_id to write_raw_bids() does write the annotations. It's just the combination of the two that causes issues, as annotations whose description doesn't appear in event_id will not be written.

[agramfort]

we have discussed this before and we had decided that we should write anything that is present in the files. so to me it's a bug fix

…

Message ID: ***@***.***>

[hoechenberger]

Yes but the question is simply how to fix it! It's clear that it's a bug :)

[sappelhoff]

Thanks for investigating, Richard!

We could of course also add special handling for all annotations for which no event_id mapping has been supplied, and we simply generate some new (arbitrary) event codes.

I like this fix:

• keep current behavior BUT
• generate new (arbitrary) event codes for annots that have no event_id mapping AND
• warn if we do that, that users please use complete their event_id instead of relying on the auto-procedure

Another option would be to keep the behavior as-is if only annotations are present (and no events and event_id are passed), and we only enforce all annotations to be listed in event_id if events and annoations are passed.

I would also be fine with this fix.

[hoechenberger]

keep current behavior BUT
generate new (arbitrary) event codes for annots that have no event_id mapping AND
warn if we do that, that users please use complete their event_id instead of relying on the auto-procedure

I'm in favor of this one! This will make things more explicit to users

[hoechenberger]

I can try to take a look sometime later today. I think this is actually an important issue and warrants a quick patch release, as users might actually be losing data in the BIDS conversion process!

[sappelhoff]

Just for the procedure of a patch release we would:

1. git cherry-pick commits from main that we want to release and put them on maint/v0.11
2. bump version, update changelog, etc. on maint/v0.11
3. build dist and push to pypi on maint/v0.11
4. build docs on v0.11 and update stable docs in gh-pages branch

right?

[hoechenberger]

Yeah or just release 0.12 for simplicity hehe

[larsoner]

Just for the procedure of a patch release we would:

This sounds like what I do for MNE-Python, yeah