MRG: Handle sidecar <> raw disparities in channels

[hoechenberger]

@agramfort feel free to take over, this seems to fix your problem

Merge checklist

Maintainer, please confirm the following before merging:

• All comments resolved
• This is not your own PR
• All CIs are happy
• PR title starts with [MRG]
• whats_new.rst is updated
• PR description includes phrase "closes <#issue-number>"

[agramfort]

thx !

you plan to add a test @hoechenberger ?

[hoechenberger]

If you think this is good, I can add a test!

@sappelhoff WDYT? Issue was that we had data with channels listed in the sidecar, which were not present in the raw. (HPI channels I believe. One could argue that these should be specified differently then -- which BIDS readily allows -- however we still have problematic datasets out in the wild we'll have to deal with...)

[sappelhoff]

looks like a good approach to me and I like the warnings. 👍

[hoechenberger]

@sappelhoff @agramfort I've added tests, good to go from my end

[hoechenberger]

the KIT test fails because after writing, channels.tsv doesn't list STI 014, but this channel is still included in the raw data (or, rather, synthesized by MNE-Python upon file reading, right?)

How shall we deal with this?

[hoechenberger]

Related:
#51

[sappelhoff]

synthesized by MNE-Python upon file reading, right?

I am not 100% sure, but I thought that we wanted to stop synthesizing stim channels in MNE-Python🤔 or was that only for some particular cases (e.g., for EEG only data)?

I think it'd be good to ignore any channels that MNE-Python synthesizes ... or stop it from automatically synthesizing them in the first place.

[hoechenberger]

@agramfort any insights on this one?

[hoechenberger]

My personal opinion is that since all events should be listed in the sidecar, MNE-BIDS could simply drop STI 014 after reading the file …

[sappelhoff]

My personal opinion is that since all events should be listed in the sidecar, MNE-BIDS could simply drop STI 014 after reading the file …

Sounds reasonable to me. If it's synthesized by MNE during reading, we wouldn't even have to log information about it. Else, an info/warning may be good.

[hoechenberger]

Sounds reasonable to me. If it's synthesized by MNE during reading, we wouldn't even have to log information about it. Else, an info/warning may be good.

Thing is, I don't know if this channel is always synthesized (and for which file formats) …

What I would propose is:
When reading data and STI 014 is not present in channels.tsv, but appears in the raw, remove it from the raw. This should be rather conservative.

Thoughts?

[sappelhoff]

LGTM, provided CIs pass, +1 to merge

[agramfort]

thx @hoechenberger