bids-standard / legacy-validator

Validator for the Brain Imaging Data Structure
https://bids-standard.github.io/legacy-validator/
MIT License
186 stars 111 forks source link

feat: Do not warn on overridden columns if meaning is unchanged #2084

Closed effigies closed 3 months ago

effigies commented 3 months ago

We had some noise in the form of

    [WARNING] TSV_COLUMN_TYPE_REDEFINED A column required in a TSV file has been redefined in a sidecar file. This redefinition is being ignored.
        participant_id
        /participants.tsv - defined in /participants.json

        onset
        /sub-001/eeg/sub-001_task-attention_events.tsv - defined in /sub-001/eeg/sub-001_task-attention_events.json
        /sub-002/eeg/sub-002_task-attention_events.tsv - defined in /sub-002/eeg/sub-002_task-attention_events.json

        duration
        /sub-001/eeg/sub-001_task-attention_events.tsv - defined in /sub-001/eeg/sub-001_task-attention_events.json
        /sub-002/eeg/sub-002_task-attention_events.tsv - defined in /sub-002/eeg/sub-002_task-attention_events.json

        trial_type
        /sub-001/eeg/sub-001_task-attention_events.tsv - defined in /sub-001/eeg/sub-001_task-attention_events.json
        /sub-002/eeg/sub-002_task-attention_events.tsv - defined in /sub-002/eeg/sub-002_task-attention_events.json

        response_time
        /sub-001/eeg/sub-001_task-attention_events.tsv - defined in /sub-001/eeg/sub-001_task-attention_events.json
        /sub-002/eeg/sub-002_task-attention_events.tsv - defined in /sub-002/eeg/sub-002_task-attention_events.json

        stim_file
        /sub-001/eeg/sub-001_task-attention_events.tsv - defined in /sub-001/eeg/sub-001_task-attention_events.json
        /sub-002/eeg/sub-002_task-attention_events.tsv - defined in /sub-002/eeg/sub-002_task-attention_events.json

    Please visit https://neurostars.org/search?q=TSV_COLUMN_TYPE_REDEFINED for existing conversations about this issue.

These were the result of column definitions containing only text descriptions or minimal, correct definitions of the column. This PR adds checks to ensure that the meaning would not change by using the provided definition instead of the schema definition, and does not raise an issue in that case.

Closes https://github.com/bids-standard/bids-validator/issues/1624.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 89.54704% with 30 lines in your changes missing coverage. Please review.

Project coverage is 88.28%. Comparing base (24e9250) to head (4ab4a9b). Report is 16 commits behind head on master.

Files Patch % Lines
bids-validator/src/schema/tables.ts 89.37% 26 Missing and 1 partial :warning:
bids-validator/src/types/filetree.ts 85.00% 3 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2084 +/- ## ========================================== + Coverage 85.69% 88.28% +2.58% ========================================== Files 91 131 +40 Lines 3782 6785 +3003 Branches 1220 1629 +409 ========================================== + Hits 3241 5990 +2749 - Misses 455 703 +248 - Partials 86 92 +6 ```

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

effigies commented 3 months ago

Reduced:

    [WARNING] TSV_COLUMN_TYPE_REDEFINED A column required in a TSV file has been redefined in a sidecar file. This redefinition is being ignored.
        onset
        /sub-001/eeg/sub-001_task-attention_events.tsv - defined in /sub-001/eeg/sub-001_task-attention_events.json
        /sub-002/eeg/sub-002_task-attention_events.tsv - defined in /sub-002/eeg/sub-002_task-attention_events.json

        duration
        /sub-001/eeg/sub-001_task-attention_events.tsv - defined in /sub-001/eeg/sub-001_task-attention_events.json
        /sub-002/eeg/sub-002_task-attention_events.tsv - defined in /sub-002/eeg/sub-002_task-attention_events.json

        response_time
        /sub-001/eeg/sub-001_task-attention_events.tsv - defined in /sub-001/eeg/sub-001_task-attention_events.json
        /sub-002/eeg/sub-002_task-attention_events.tsv - defined in /sub-002/eeg/sub-002_task-attention_events.json

    Please visit https://neurostars.org/search?q=TSV_COLUMN_TYPE_REDEFINED for existing conversations about this issue.

These columns override unit: s with "Unit": "seconds". I don't want to try to reconcile those, and they should use CMIXF-12, anyway.