-
Notifications
You must be signed in to change notification settings - Fork 190
Fix multiple validation errors for EMG coordsystem and electrode files #2279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix multiple validation errors for EMG coordsystem and electrode files #2279
Conversation
6148cc8 to
74f819b
Compare
- Exclude coordsystem files from SIDECAR_WITHOUT_DATAFILE error - Add subject: optional to coordsystem__emg for root-level placement - Add coordsystem__emg_root rule for root-level files - Add conditional uniqueness for EMG electrodes based on group column - Update test for optional subject patterns
efde93f to
37f5f38
Compare
|
While testing EMG datasets on NEMAR (like nm000104, and nm000105), I noticed a couple of issues. I tested the validation using the latest pull, These fixes seem to eliminate the issues, you can double check via: |
|
it looks like you're using the current stable version of the validator ( Here's my output for nm000104: DetailsHere's my output for nm000105: Details |
I think all of these were addressed in bids-standard/bids-validator#268 The fact that e.g., EMGTwoWristbands already has a root-level coordsys.json, and all CIs passed on bids-standard/bids-examples#480, indicates that these issues were already addressed. Also, |
effigies
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @drammock. @neuromechanist I think you may have needed to reload your validator with deno cache --reload jsr:@bids/validator. (You can also add --reload to your deno run command.)
| - extension == ".json" | ||
| - suffix != "coordsystem" || modality != "emg" | ||
| # ↑ EMG allows multiple coordsys files per data file (distinguished via `space` entity) | ||
| - suffix != "coordsystem" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably correct, but these selectors are not actually used by the validator. The SIDECAR_WITHOUT_DATAFILE error is a separate check after walking the whole dataset, so the context isn't available for selecting, at least in the JS implementation.
| - emg | ||
| entities: | ||
| $ref: rules.files.raw.channels.coordsystem__eeg.entities | ||
| subject: optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Subject is always optional for TSV or JSON files, due to the inheritance principle.
| initial_columns: | ||
| - name__electrodes | ||
| - x | ||
| - y | ||
| columns: | ||
| name__electrodes: required | ||
| x: required | ||
| y: required | ||
| z: optional | ||
| coordinate_system: recommended | ||
| type__electrodes: recommended | ||
| material: recommended | ||
| impedance: recommended | ||
| group__emg: recommended | ||
| index_columns: [name__electrodes] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional index and initial columns were implemented in bids-standard/bids-validator#243.
Summary
Fix multiple validation errors for EMG coordsystem and electrode files.
Changes
1. Fix SIDECAR_WITHOUT_DATAFILE for coordsystem files (errors.yaml)
Changed selector from:
- suffix != "coordsystem" || modality != "emg"To:
- suffix != "coordsystem"Reason: Coordsystem files can be shared via the inheritance principle and may not have direct data files. This applies to all modalities, not just EMG. The simpler selector excludes all coordsystem files from this check. Also, it seems that when in root, the error comes up before modality is checked, therefore, the error is being raised.
2. Allow root-level EMG coordsystem files (channels.yaml)
subject: optionaltocoordsystem__emgrulecoordsystem__emg_rootrule withdatatypes: []for root-level placementReason: EMG datasets can place coordsystem files at root level (no subject entity) to share coordinate system definitions across all subjects via inheritance.
3. Conditional uniqueness for EMG electrodes (emg.yaml)
Split
EMGElectrodesinto two rules:EMGElectrodes: When nogroupcolumn exists, electrodenamemust be uniqueEMGElectrodes__grouped: Whengroupcolumn exists, the combination of(name, group)must be uniqueUses selectors
!columns.groupandcolumns.groupto determine which rule applies.Reason: EMG datasets with bilateral recordings (left/right arm) may have the same electrode names (e.g., E0-E15) repeated across different groups. The uniqueness constraint should be conditional on whether grouping is used.
4. Update test for optional subject (test_render_text.py)
Added handling for patterns with optional subject entity in filename template rendering test.
Reason: Subject is required for coordsystem files in other modalities (EEG, MEG, NIRS, iEEG), but optional for EMG to allow root-level placement. When the schema renders filename templates, the output shows
[sub-<label>]for optional subject patterns. The test parses this rendered text without modality context, so it cannot distinguish EMG patterns from other modalities. We skip[sub-<label>]patterns in the test since they represent valid EMG coordsystem files.Valid coordsystem patterns now supported
space-leftForearm_coordsystem.json(root level, with space entity)sub-01/emg/sub-01_coordsystem.json(subject level)sub-01/emg/sub-01_space-hand_coordsystem.json(subject level with space)Related