Skip to content
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

Add "master-deno" flavor of validator to use as well #435

Merged
merged 25 commits into from
Jun 6, 2024

Conversation

yarikoptic
Copy link
Contributor

Sits on top of #434 to have it correct here. Relevant to this PR changes are only to workflow

@yarikoptic yarikoptic marked this pull request as ready for review April 19, 2024 21:18
@yarikoptic
Copy link
Contributor Author

yarikoptic commented Apr 19, 2024

Although it fails deno validator I think it is ready for review.
For deno validator I "manually" filter out warnings and error for EMPTY_FILE and adjusted output so now it looks more condensed and to the point (edit clarification: files_1 is the "1st file of files which might have many"):

❯ ./run_tests.sh
/tmp/bids-validator/bids-validator
Validating dataset 7t_trt/: Running  bids-validator 7t_trt --json --ignoreNiftiHeaders
...
Validating dataset docs/: skipping validation
Validating dataset ds000001-fmriprep/: skipping validation
Validating dataset ds000117/: Running  bids-validator ds000117 --json --ignoreNiftiHeaders
  [
    {
      "key": "CHECK_ERROR",
      "severity": "error",
      "reason": "generic place holder for errors from failed `checks` evaluated from schema.",
      "files_1": [
        {
          "name": "sub-01_ses-meg_task-facerecognition_run-01_meg.fif",
          "path": "/sub-01/ses-meg/meg/sub-01_ses-meg_task-facerecognition_run-01_meg.fif",
          "evidence": "schema.rules.checks.references.AssociatedEmptyRoomString"
        }
      ]
    }
  ]
Validating dataset ds000246/: Running  bids-validator ds000246 --json --ignoreNiftiHeaders
...
Datasets failed validation:  ds000117/ ds000248/ eeg_ds003645s_hed_demo/ ieeg_epilepsy/ ieeg_epilepsy_ecog/ ieeg_epilepsyNWB/ ieeg_visual_multimodal/
full dump from my local laptop but seems the same on CI
❯ ./run_tests.sh
/tmp/bids-validator/bids-validator
Validating dataset 7t_trt/: Running  bids-validator 7t_trt --json --ignoreNiftiHeaders
Validating dataset asl001/: Running  bids-validator asl001 --json --ignoreNiftiHeaders
Validating dataset asl002/: Running  bids-validator asl002 --json --ignoreNiftiHeaders
Validating dataset asl003/: Running  bids-validator asl003 --json --ignoreNiftiHeaders
Validating dataset asl004/: Running  bids-validator asl004 --json --ignoreNiftiHeaders
Validating dataset asl005/: Running  bids-validator asl005 --json --ignoreNiftiHeaders
Validating dataset docs/: skipping validation
Validating dataset ds000001-fmriprep/: skipping validation
Validating dataset ds000117/: Running  bids-validator ds000117 --json --ignoreNiftiHeaders
  [
    {
      "key": "CHECK_ERROR",
      "severity": "error",
      "reason": "generic place holder for errors from failed `checks` evaluated from schema.",
      "files_1": [
        {
          "name": "sub-01_ses-meg_task-facerecognition_run-01_meg.fif",
          "path": "/sub-01/ses-meg/meg/sub-01_ses-meg_task-facerecognition_run-01_meg.fif",
          "evidence": "schema.rules.checks.references.AssociatedEmptyRoomString"
        }
      ]
    }
  ]
Validating dataset ds000246/: Running  bids-validator ds000246 --json --ignoreNiftiHeaders
Validating dataset ds000247/: Running  bids-validator ds000247 --json --ignoreNiftiHeaders
Validating dataset ds000248/: Running  bids-validator ds000248 --json --ignoreNiftiHeaders
  [
    {
      "key": "MISSING_REQUIRED_ENTITY",
      "severity": "error",
      "reason": "Missing required entity for files with this suffix.",
      "files_1": [
        {
          "name": "sub-01_acq-crosstalk_meg.fif",
          "path": "/sub-01/meg/sub-01_acq-crosstalk_meg.fif",
          "evidence": "task missing from rule rules.files.deriv.preprocessed_data.meg_meg_common"
        }
      ]
    }
  ]
Validating dataset ds001/: Running  bids-validator ds001 --json --ignoreNiftiHeaders
Validating dataset ds002/: Running  bids-validator ds002 --json --ignoreNiftiHeaders
Validating dataset ds003/: Running  bids-validator ds003 --json --ignoreNiftiHeaders
Validating dataset ds004332/: Running  bids-validator ds004332 --json --ignoreNiftiHeaders
Validating dataset ds005/: Running  bids-validator ds005 --json --ignoreNiftiHeaders
Validating dataset ds006/: Running  bids-validator ds006 --json --ignoreNiftiHeaders
Validating dataset ds007/: Running  bids-validator ds007 --json --ignoreNiftiHeaders
Validating dataset ds008/: Running  bids-validator ds008 --json --ignoreNiftiHeaders
Validating dataset ds009/: Running  bids-validator ds009 --json --ignoreNiftiHeaders
Validating dataset ds011/: Running  bids-validator ds011 --json --ignoreNiftiHeaders
Validating dataset ds051/: Running  bids-validator ds051 --json --ignoreNiftiHeaders
Validating dataset ds052/: Running  bids-validator ds052 --json --ignoreNiftiHeaders
Validating dataset ds101/: Running  bids-validator ds101 --json --ignoreNiftiHeaders
Validating dataset ds102/: Running  bids-validator ds102 --json --ignoreNiftiHeaders
Validating dataset ds105/: Running  bids-validator ds105 --json --ignoreNiftiHeaders
Validating dataset ds107/: Running  bids-validator ds107 --json --ignoreNiftiHeaders
Validating dataset ds108/: Running  bids-validator ds108 --json --ignoreNiftiHeaders
Validating dataset ds109/: Running  bids-validator ds109 --json --ignoreNiftiHeaders
Validating dataset ds110/: Running  bids-validator ds110 --json --ignoreNiftiHeaders
Validating dataset ds113b/: Running  bids-validator ds113b --json --ignoreNiftiHeaders
Validating dataset ds114/: Running  bids-validator ds114 --json --ignoreNiftiHeaders
Validating dataset ds116/: Running  bids-validator ds116 --json --ignoreNiftiHeaders
Validating dataset ds210/: Running  bids-validator ds210 --json --ignoreNiftiHeaders
Validating dataset eeg_cbm/: Running  bids-validator eeg_cbm --json --ignoreNiftiHeaders
Validating dataset eeg_ds000117/: Running  bids-validator eeg_ds000117 --json --ignoreNiftiHeaders
Validating dataset eeg_ds003645s_hed_demo/: Running  bids-validator eeg_ds003645s_hed_demo --json --ignoreNiftiHeaders
  [
    {
      "key": "NOT_INCLUDED",
      "severity": "error",
      "reason": "Files with such naming scheme are not part of BIDS specification. This error is most commonly caused by typos in file names that make them not BIDS compatible. Please consult the specification and make sure your files are named correctly. If this is not a file naming issue (for example when including files not yet covered by the BIDS specification) you should include a \".bidsignore\" file in your dataset (see https://github.com/bids-standard/bids-validator#bidsignore for details). Please note that derived (processed) data should be placed in /derivatives folder and source data (such as DICOMS or behavioural logs in proprietary formats) should be placed in the /sourcedata folder.",
      "files_1": [
        {
          "name": "KSSSleep.json",
          "path": "/phenotype/KSSSleep.json"
        }
      ]
    },
    {
      "key": "ALL_FILENAME_RULES_HAVE_ISSUES",
      "severity": "error",
      "reason": "Multiple filename rules were found as potential matches. All of them had at least one issue during filename validation.",
      "files_1": [
        {
          "name": "sub-004_ses-1_task-FacePerception_coordsystem.json",
          "path": "/sub-004/ses-1/sub-004_ses-1_task-FacePerception_coordsystem.json",
          "evidence": "Rules that matched with issues: rules.files.raw.channels.coordsystem, rules.files.raw.channels.coordsystem__eeg"
        }
      ]
    },
    {
      "key": "JSON_KEY_REQUIRED",
      "severity": "error",
      "reason": "A data file's JSON sidecar is missing a key listed as required.",
      "files_1": [
        {
          "name": "sub-004_ses-1_task-FacePerception_run-1_eeg.set",
          "path": "/sub-004/ses-1/eeg/sub-004_ses-1_task-FacePerception_run-1_eeg.set",
          "evidence": "missing SoftwareFilters as per schema.rules.sidecars.eeg.EEGRequired"
        }
      ]
    },
    {
      "key": "ENTITY_NOT_IN_RULE",
      "severity": "error",
      "reason": "Entity not listed as required or optional for files with this suffix",
      "files_1": [
        {
          "name": "sub-004_ses-1_task-FacePerception_run-1_electrodes.tsv",
          "path": "/sub-004/ses-1/eeg/sub-004_ses-1_task-FacePerception_run-1_electrodes.tsv",
          "evidence": "task, run not in rule rules.files.raw.channels.electrodes"
        }
      ]
    }
  ]
Validating dataset eeg_ds003645s_hed_library/: Running  bids-validator eeg_ds003645s_hed_library --json --ignoreNiftiHeaders
Validating dataset eeg_face13/: Running  bids-validator eeg_face13 --json --ignoreNiftiHeaders
Validating dataset eeg_matchingpennies/: Running  bids-validator eeg_matchingpennies --json --ignoreNiftiHeaders
Validating dataset eeg_rest_fmri/: Running  bids-validator eeg_rest_fmri --json --ignoreNiftiHeaders
Validating dataset eeg_rishikesh/: Running  bids-validator eeg_rishikesh --json --ignoreNiftiHeaders
Validating dataset fnirs_automaticity/: Running  bids-validator fnirs_automaticity --json --ignoreNiftiHeaders
Validating dataset fnirs_tapping/: Running  bids-validator fnirs_tapping --json --ignoreNiftiHeaders
Validating dataset genetics_ukbb/: Running  bids-validator genetics_ukbb --json --ignoreNiftiHeaders
Validating dataset hcp_example_bids/: Running  bids-validator hcp_example_bids --json --ignoreNiftiHeaders
Validating dataset ieeg_epilepsy/: Running  bids-validator ieeg_epilepsy --json --ignoreNiftiHeaders
  [
    {
      "key": "CHECK_ERROR",
      "severity": "error",
      "reason": "generic place holder for errors from failed `checks` evaluated from schema.",
      "files_1": [
        {
          "name": "sub-01_ses-postimp_space-IXI549Space_coordsystem.json",
          "path": "/sub-01/ses-postimp/ieeg/sub-01_ses-postimp_space-IXI549Space_coordsystem.json",
          "evidence": "schema.rules.checks.references.DatasetRelativeIntendedForString"
        }
      ]
    }
  ]
Validating dataset ieeg_epilepsy_ecog/: Running  bids-validator ieeg_epilepsy_ecog --json --ignoreNiftiHeaders
  [
    {
      "key": "CHECK_ERROR",
      "severity": "error",
      "reason": "generic place holder for errors from failed `checks` evaluated from schema.",
      "files_1": [
        {
          "name": "sub-ecog01_ses-postimp_space-IXI549Space_coordsystem.json",
          "path": "/sub-ecog01/ses-postimp/ieeg/sub-ecog01_ses-postimp_space-IXI549Space_coordsystem.json",
          "evidence": "schema.rules.checks.references.DatasetRelativeIntendedForString"
        }
      ]
    }
  ]
Validating dataset ieeg_epilepsyNWB/: Running  bids-validator ieeg_epilepsyNWB --json --ignoreNiftiHeaders
  [
    {
      "key": "CHECK_ERROR",
      "severity": "error",
      "reason": "generic place holder for errors from failed `checks` evaluated from schema.",
      "files_1": [
        {
          "name": "sub-01_ses-postimp_space-IXI549Space_coordsystem.json",
          "path": "/sub-01/ses-postimp/ieeg/sub-01_ses-postimp_space-IXI549Space_coordsystem.json",
          "evidence": "schema.rules.checks.references.DatasetRelativeIntendedForString"
        }
      ]
    }
  ]
Validating dataset ieeg_filtered_speech/: Running  bids-validator ieeg_filtered_speech --json --ignoreNiftiHeaders
Validating dataset ieeg_motorMiller2007/: Running  bids-validator ieeg_motorMiller2007 --json --ignoreNiftiHeaders
Validating dataset ieeg_visual/: Running  bids-validator ieeg_visual --json --ignoreNiftiHeaders
Validating dataset ieeg_visual_multimodal/: Running  bids-validator ieeg_visual_multimodal --json --ignoreNiftiHeaders
  [
    {
      "key": "CHECK_ERROR",
      "severity": "error",
      "reason": "generic place holder for errors from failed `checks` evaluated from schema.",
      "files_1": [
        {
          "name": "sub-som682_ses-somecog01_coordsystem.json",
          "path": "/sub-som682/ses-somecog01/ieeg/sub-som682_ses-somecog01_coordsystem.json",
          "evidence": "schema.rules.checks.references.DatasetRelativeIntendedForString"
        }
      ]
    }
  ]
Validating dataset micr_SEM/: Running  bids-validator micr_SEM --json --ignoreNiftiHeaders
Validating dataset micr_SEMzarr/: Running  bids-validator micr_SEMzarr --json --ignoreNiftiHeaders
Validating dataset micr_SPIM/: Running  bids-validator micr_SPIM --json --ignoreNiftiHeaders
Validating dataset motion_dualtask/: Running  bids-validator motion_dualtask --json --ignoreNiftiHeaders
Validating dataset motion_spotrotation/: Running  bids-validator motion_spotrotation --json --ignoreNiftiHeaders
Validating dataset motion_systemvalidation/: Running  bids-validator motion_systemvalidation --json --ignoreNiftiHeaders
Validating dataset pet001/: Running  bids-validator pet001 --json --ignoreNiftiHeaders
Validating dataset pet002/: Running  bids-validator pet002 --json --ignoreNiftiHeaders
Validating dataset pet003/: Running  bids-validator pet003 --json --ignoreNiftiHeaders
Validating dataset pet004/: Running  bids-validator pet004 --json --ignoreNiftiHeaders
Validating dataset pet005/: Running  bids-validator pet005 --json --ignoreNiftiHeaders
Validating dataset qmri_irt1/: Running  bids-validator qmri_irt1 --json --ignoreNiftiHeaders
Validating dataset qmri_megre/: Running  bids-validator qmri_megre --json --ignoreNiftiHeaders
Validating dataset qmri_mese/: Running  bids-validator qmri_mese --json --ignoreNiftiHeaders
Validating dataset qmri_mp2rage/: Running  bids-validator qmri_mp2rage --json --ignoreNiftiHeaders
Validating dataset qmri_mp2rageme/: Running  bids-validator qmri_mp2rageme --json --ignoreNiftiHeaders
Validating dataset qmri_mpm/: Running  bids-validator qmri_mpm --json --ignoreNiftiHeaders
Validating dataset qmri_mtsat/: Running  bids-validator qmri_mtsat --json --ignoreNiftiHeaders
Validating dataset qmri_qsm/: Running  bids-validator qmri_qsm --json --ignoreNiftiHeaders
Validating dataset qmri_sa2rage/: Running  bids-validator qmri_sa2rage --json --ignoreNiftiHeaders
Validating dataset qmri_tb1tfl/: Running  bids-validator qmri_tb1tfl --json --ignoreNiftiHeaders
Validating dataset qmri_vfa/: Running  bids-validator qmri_vfa --json --ignoreNiftiHeaders
Validating dataset synthetic/: validating NIfTI headers. 
Running  bids-validator synthetic --json
Validating dataset tools/: skipping validation
Datasets failed validation:  ds000117/ ds000248/ eeg_ds003645s_hed_demo/ ieeg_epilepsy/ ieeg_epilepsy_ecog/ ieeg_epilepsyNWB/ ieeg_visual_multimodal/
./run_tests.sh  24.26s user 2.91s system 121% cpu 22.436 total

…ename

to hopefully become compatible with windows environment
@effigies
Copy link
Contributor

@yarikoptic
Copy link
Contributor Author

Thank you @effigies for bringing it over the green line! So the summary of the issue is that on Windows /usr/local/bin was not present/in the PATH?

@effigies
Copy link
Contributor

Yeah. Though /usr/bin is. Maybe the cleaner solution is just to drop the file there, rather than mucking around with the PATH and then having to stick with bash after doing so.

@yarikoptic
Copy link
Contributor Author

oh, then indeed let's just drop into /usr/bin -- no need to be "kosher" ... will push now.

It is just CI.  Then we can avoid messing around with PATH for Windows
@yarikoptic
Copy link
Contributor Author

apparently trying to modify something I have no permission to modify, so might need sudo?
Run set -x
+ pushd ..
+ git clone --depth 1 https://github.com/bids-standard/bids-validator
~/work/bids-examples ~/work/bids-examples/bids-examples
Cloning into 'bids-validator'...
+ VALIDATOR=/usr/bin/bids-validator
+ echo -e '#!/bin/sh\n/home/runner/work/bids-examples/bids-validator/bids-validator/bids-validator-deno "$@"'
/home/runner/work/_temp/0c554568-7be5-4bc1-ae0d-02eb00ba728d.sh: line 5: /usr/bin/bids-validator: Permission denied

on linux and on OSX:

+ pushd ..
18
~/work/bids-examples ~/work/bids-examples/bids-examples
19
+ git clone --depth 1 https://github.com/bids-standard/bids-validator
20
Cloning into 'bids-validator'...
21
+ VALIDATOR=/usr/bin/bids-validator
22
+ echo -e '#!/bin/sh\n/Users/runner/work/bids-examples/bids-validator/bids-validator/bids-validator-deno "$@"'
23
/Users/runner/work/_temp/5706bc01-fd0e-4e3d-9711-5bc2fd805a7c.sh: line 5: /usr/bin/bids-validator: Read-only file system

@yarikoptic
Copy link
Contributor Author

all platforms have in PATH some ../.deno/bin. @effigies @nellh @rwblair , do you know some deno command to give me such a path? chatgpt only came up with some code to compile/run... bleh..

Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@effigies
Copy link
Contributor

effigies commented Jun 5, 2024

Not sure why Windows is succeeding. Possibly we aren't normalizing path separators, so no rules are matching.

Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@yarikoptic
Copy link
Contributor Author

yeay! overall it seems to work as "this PR functionality is ready". We just have now a few cases where it is either new validator needs to be fixed up, or datasets, or BIDS standard. E.g.

inconsistency in age spec in BIDS:

image

as "89+" is not a number. To fix that, and harmonize for having "Units" I proposed

but it got stuck in review... alternative would be to adjust BIDS & schema to always require a number (or "n/a" as it is generally allowed AFAIK for missing values in .tsv!) and treat either 89 or 90 (without any +) as that "boundary condition".

Only genetics_ukbb/participants.tsv has those

legitimately incorrect example datasets

e.g.

❯ cat ieeg_epilepsy/participants.tsv
participant_id	age	sex
sub-01	XX	m%  

I guess we should fix that in separate PRs right?

@effigies
Copy link
Contributor

effigies commented Jun 6, 2024

Yeah, @rwblair is working on shifting these "recommended" columns to warnings when they do not match (bids-standard/bids-validator#1979).

And I have bids-standard/bids-specification#1838 which is somewhat related in that it will give us a baseline for validating based on JSON-sidecar column definitions.

@yarikoptic
Copy link
Contributor Author

meanwhile sent a few fixes in

@yarikoptic
Copy link
Contributor Author

yeay -- 1 approval achieved. @Remi-Gau ?

@effigies effigies merged commit 09df87f into bids-standard:master Jun 6, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants