bids-standard / bids-specification

Brain Imaging Data Structure (BIDS) Specification
https://bids-specification.readthedocs.io/
Creative Commons Attribution 4.0 International
279 stars 163 forks source link

[FIX] Allow (but discourage) task, acquisition and run entities for coordsystem.json and electrodes.tsv #1888

Closed effigies closed 1 month ago

effigies commented 3 months ago

We've run into a situation where the regular validator regular expressions were written more laxly than the specification, leading to datasets that use entities that are against the spec but perfectly interpretable. In particular, the task entity has been permitted for coordsystem.json across EEG, iEEG, MEG and NIRS.

Following the pattern established in #1722, instead of marking these datasets invalid with a schema-based validator, we relax the spec but add warnings.

cc @bids-standard/raw-electrophys and @rob-luke @lpollonini (we should make a NIRS team...).

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.06%. Comparing base (0e506f0) to head (c0915eb).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1888 +/- ## ======================================= Coverage 88.06% 88.06% ======================================= Files 16 16 Lines 1391 1391 ======================================= Hits 1225 1225 Misses 166 166 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

effigies commented 1 month ago

@dorahermes @VisLab WDYT?

dorahermes commented 1 month ago

Just noting that if a _task- entity is allowed for an _coordsystem.json it should also be allowed for an _electrodes.tsv.

effigies commented 1 month ago

Good catch. task, acquisition and run were all permitted in the legacy validator for both electrodes.tsv and coordsystem.json, and all six combinations exist on OpenNeuro. For consistency, I've updated this PR to treat these all the same.