search

bids-standard / legacy-validator

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

`error: MULTIPLE_INHERITABLE_FILES` should mention the problematic json files #2153

Closed Remi-Gau closed 3 weeks ago

Remi-Gau commented 3 weeks ago

Pointed out to me by @bthirion

The error message for multiple inheritance should ideally also point to the problematic JSON files and not just to the data files affected by it.

I modified the 7t_trt example https://github.com/bids-standard/bids-examples/tree/master/7t_trt

to have files with clashing acq-fullbrain_bold.json files

$ ls  -l 7t_trt/*.json
-rw-rw-r-- 1 remi remi 1328 oct.  10 16:12 7t_trt/acq-fullbrain_bold.json <-- HERE
-rw-rw-r-- 1 remi remi   53 oct.  21  2022 7t_trt/dataset_description.json
-rw-rw-r-- 1 remi remi  526 nov.   9  2023 7t_trt/participants.json
-rw-rw-r-- 1 remi remi 1328 oct.  21  2022 7t_trt/task-rest_acq-fullbrain_bold.json  <-- HERE
-rw-rw-r-- 1 remi remi  113 oct.  21  2022 7t_trt/task-rest_acq-fullbrain_run-1_physio.json
-rw-rw-r-- 1 remi remi  113 oct.  21  2022 7t_trt/task-rest_acq-fullbrain_run-2_physio.json
-rw-rw-r-- 1 remi remi  887 oct.  21  2022 7t_trt/task-rest_acq-prefrontal_bold.json
-rw-rw-r-- 1 remi remi  113 oct.  21  2022 7t_trt/task-rest_acq-prefrontal_physio.json

web browser validator gives (amongst other things)

Multiple files in a directory were found to be valid candidates for inheritance.

    /sub-05/ses-1/func/sub-05_ses-1_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-05/ses-1/func/sub-05_ses-1_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-05/ses-2/func/sub-05_ses-2_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-05/ses-2/func/sub-05_ses-2_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-18/ses-1/func/sub-18_ses-1_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-18/ses-1/func/sub-18_ses-1_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-18/ses-2/func/sub-18_ses-2_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-18/ses-2/func/sub-18_ses-2_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-20/ses-1/func/sub-20_ses-1_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-20/ses-1/func/sub-20_ses-1_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-20/ses-2/func/sub-20_ses-2_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-20/ses-2/func/sub-20_ses-2_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-09/ses-1/func/sub-09_ses-1_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-09/ses-1/func/sub-09_ses-1_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-09/ses-2/func/sub-09_ses-2_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-09/ses-2/func/sub-09_ses-2_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-08/ses-1/func/sub-08_ses-1_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-08/ses-1/func/sub-08_ses-1_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-08/ses-2/func/sub-08_ses-2_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-08/ses-2/func/sub-08_ses-2_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-02/ses-1/func/sub-02_ses-1_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-02/ses-1/func/sub-02_ses-1_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-02/ses-2/func/sub-02_ses-2_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-02/ses-2/func/sub-02_ses-2_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-03/ses-1/func/sub-03_ses-1_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-03/ses-1/func/sub-03_ses-1_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-03/ses-2/func/sub-03_ses-2_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-03/ses-2/func/sub-03_ses-2_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-21/ses-1/func/sub-21_ses-1_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-21/ses-1/func/sub-21_ses-1_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-21/ses-2/func/sub-21_ses-2_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-21/ses-2/func/sub-21_ses-2_task-rest_acq-fullbrain_run-1_bold.nii.gz
    /sub-13/ses-1/func/sub-13_ses-1_task-rest_acq-fullbrain_run-2_bold.nii.gz
    /sub-13/ses-1/func/sub-13_ses-1_task-rest_acq-fullbrain_run-1_bold.nii.gz

I would expect for the error message to also mention:

effigies commented 3 weeks ago

This should be present in the JSON errors:

https://github.com/bids-standard/bids-validator/blob/c52a6712fd0067744d28b426f7f120ccc0bc3b1f/bids-validator/src/files/inheritance.ts#L36-L40

So we need to add the affects to rendering.

effigies commented 3 weeks ago

Well, maybe let's think more about the best way to handle this.

Our current issue model only permits a unique location, as there should be a single place to fix the error. When an invalid value appears in the JSON file, the location is the JSON file and it affects any data files that inherit it. If it's absent, the location is the data file, since we don't want to try to guess where it should have been written.

The location of this issue is not the BOLD file, but the directory containing the two inheritable files. It might be simplest, though, to call the location the first one, say that the affected files are the data files, and include the list of competing matches in the message.

cc @rwblair

rwblair commented 3 weeks ago

Message now tells offending json files, but check is still done on each file that inheritance sidecar merging is done so a large number of errors with the same message are generated:

        [ERROR] MULTIPLE_INHERITABLE_FILES Multiple files in a directory were found to be valid candidates for inheritance.
                /acq-fullbrain_bold.json - Candidate files: /acq-fullbrain_bold.json,/task-rest_acq-fullbrain_bold.json
                /acq-fullbrain_bold.json - Candidate files: /acq-fullbrain_bold.json,/task-rest_acq-fullbrain_bold.json

                86 more files with the same issue

We are working on reducing output with the exact same message.

Remi-Gau commented 3 weeks ago

awesome thanks @effigies and @rwblair for the quick fix!!