mne-tools / mne-bids

MNE-BIDS is a Python package that allows you to read and write BIDS-compatible datasets with the help of MNE-Python.
https://mne.tools/mne-bids/
BSD 3-Clause "New" or "Revised" License
133 stars 88 forks source link

'trial_type' column with only 'n/a' will fail to extract any events even if 'value' column have the trigger codes #947

Open weiyongxu opened 2 years ago

weiyongxu commented 2 years ago

https://github.com/mne-tools/mne-bids/blob/2ef3516ba6ba54506901a773204f5f88adff224f/mne_bids/read.py#L378

Hi,

I have a BIDS dataset which has events.tsv looks something like this: MNE-BIDS fails to extract any events/annotation from the events.tsv file:

onset duration sample trial_type response_time stim_file value 111.6972656250 0.0000000000 57189 n/a n/a n/a 1040 114.6972656250 0.0000000000 58725 n/a n/a n/a 2049 117.7617187500 0.0000000000 60294 n/a n/a n/a 1040 ......

I think its's bacause the 'trial_type' column has only 'n/a', so the code will drop all the events and will not look further for the 'value' columns.

From the BIDS document here :https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/05-task-events.html both trial type and value are optional, so I guess even if the trial_type only has 'n/a', we still should check the 'value' column for possible events?

welcome[bot] commented 2 years ago

Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

hoechenberger commented 2 years ago

Hello @weiyongxu and thanks for reporting this issue! You're right, we should utilize the values even if no trial types are provided.

FWIW, the specification declares the entire trial_type column optional, meaning it can be missing entirely.

It does NOT say that n/a means that no trial type is provided. Meaning, as I read the spec in a literal way: if trial_type is "n/a", then the name of that trial type is "n/a" 😅

@sappelhoff any thoughts on this?

sappelhoff commented 2 years ago

IMHO: