channels.tsv on ds000117

[agramfort]

the ds000117 contains a channels.tsv file but it's one file for all runs of a subject and it's not in the meg folder:

https://openneuro.org/datasets/ds000117/versions/1.0.5/file-display/sub-01:ses-meg:sub-01_ses-meg_task-facerecognition_channels.tsv

as the consequence read_raw_bids does not find the file and therefore cannot read the bad channels or the fact the EEG061 and EEG062 are EOG and EEG063 is ECG.

I ran the validator on the dataset and it does not complain about this.

@hoechenberger @sappelhoff @alexrockhill is it a bug of the dataset and validator or mne-bids?

presently I am stuck...

[alexrockhill]

I think probably mne-bids should use that channels sidecar and it's a bug/not-yet-implemented on the mne-bids side. I didn't write that part so I'd have to look into it a bit more but AFAIK the channels does not use hierarchy and just looks for a file at the data level.

[alexrockhill]

From the code, it looks like it should work. If you do mne_bids.path._find_matching_sidecar(bids_path, suffix='channels', extension='.tsv') do you get a the file you linked?

[agramfort]

the search str used is:

".../ds000117/sub-01/**/meg/sub-01_ses-meg*channels.tsv"

so it looks inside "meg" and not in there. It's one level up...

[agramfort]

I ended up moving the files to the meg folder for now...

[alexrockhill]

Maybe mne-bids should search without datatype if datatype doesn't return a match. And then maybe without subject even...

[sappelhoff]

I think it's a bug in the dataset, see also bids-standard/bids-examples#296

[alexrockhill]

Do you think there would be harm in being more permissive with mne-bids and applying the hierarchical structure even if it's not supported by BIDS? As I've seen, not all datasets fit cleanly into BIDS yet and sometimes this kind of thing is helpful to being maximally flexible as long as the lowest level match is used first.

[hoechenberger]

Do you think there would be harm in being more permissive with mne-bids and applying the hierarchical structure even if it's not supported by BIDS?

We don't even really apply the hierarchical structure for BIDS-compliant datasets, and I'm one of those who believe that the hierarchical inheritance principle was a big mistake and should be killed in a future BIDS version 😅 But if you want to give its implementation a shot – sure, and good luck 🥸

[adam2392]

Also not a fan of hierarchical everything. Makes implementation a complex hell.

[alexrockhill]

Haha, well maybe I'll try a PR if I have a bit, I think it can be elegant. Maybe more opinions will come to the defense of hierarchical data structures. They sure do like it in BIDS.

[sappelhoff]

if you want a deep read on suggested changes to the inheritance principle, you can also see this: bids-standard/bids-specification#1003 and comment with your opinion :)

I think I'd be okay with an implementation of the inheritance principle in mne-bids ... but I would be against implementing support for stuff that's not allowed in BIDS

[alexrockhill]

I agree but I think an important caveat is that if it passes the BIDS-validator and has for some time, even if it's not part of the BIDS-specification, I think that should be supported for backward compatibility for as long as possible.

[jasmainak]

inheritance principle is necessary for folks who manually curate the datasets since sometimes it's easier to edit one file than multiple files. For jsons you'd have to get a list of dictionaries across levels and merge them (with lower levels taking precedence). I don't know how it would work for tsv though :)

[sappelhoff]

Afaik, inheritance is not implemented for tsvs (it's not implemented for "data" tsvs ... but it IS implemented for metadata tsvs ... it'd difficult when one would also encode metadata in a data tsv file -- which is possible due to being able to add arbitrary columns to any tsv)

[hoechenberger]

I agree but I think an important caveat is that if it passes the BIDS-validator and has for some time, even if it's not part of the BIDS-specification, I think that should be supported for backward compatibility for as long as possible.

In fact, we do support some things that are not in compliance with (the most recent revisions of) the specs. So, yeah, I'm okay with being lenient in certain cases, but we'll have to be careful not to allow for too much sloppiness :)

[alexrockhill]

Afaik, inheritance is not implemented for tsvs

Yes, this is a tricky case because some tsvs are data (i.e. _beh.tsv) and some are sidecars (e.g. _channels.tsv). I think the sidecar designation is more important than the file extension in determining inheritance and IMO inheriting sidecar files that are repeated across subjects/sessions/runs is much more DRY and therefore preferable.

[sappelhoff]

please also see: https://groups.google.com/g/bids-discussion/c/QmwChe4IREY