Update validator.yml to fix maintenance issues (dependencies, disk space, error throwing)
#168
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will make it so that simple errors won't stop other checks, while also making it easier for me to focus on the "core" changes.
joshuacwnewton
commented
Jun 27, 2024
Comment on lines
80
to
+101
| - name: Checking BIDS compliance | ||
| run: bids-validator --verbose ./ | ||
| if: always() | ||
| run: | | ||
| bids-validator --verbose ./ | tee bids.txt | ||
| printf "\nShort summary:\n\n" | ||
| # FIXME: This should be replaced with bids-validator settings: A) throw error on warning or B) warning ignores | ||
| ! grep "\[WARN\]\|\[ERR\]" bids.txt # ! -> Reverse error code so "found == error thrown" | ||
|
|
||
| - name: Checking acquisition parameters | ||
| run: sg_params_checker -path-in ./ | ||
| if: always() | ||
| run: | | ||
| sg_params_checker -path-in ./ | tee params.txt | ||
| # FIXME: Checking the output like this should be replaced with SG throwing actual errors | ||
| ! grep "WARNING" params.txt # ! -> Reverse error code so "found == error thrown" | ||
|
|
||
| - name: Checking data consistency | ||
| run: sg_check_data_consistency -path-in ./ | ||
| if: always() | ||
| run: | | ||
| sg_check_data_consistency -path-in ./ | tee consistency.txt | ||
| # FIXME: Checking the output like this should be replaced with SG throwing actual errors | ||
| ! grep "Warning\|Missing" consistency.txt | ||
| grep "all good" consistency.txt |
There was a problem hiding this comment.
Note: The failures on this PR are to be expected, because all 3 of these checks throw warnings/errors:
- BIDS validation: Link
ERR: code 1 - NOT_INCLUDEDWARNING: code 38 - INCONSISTENT_SUBJECTSWARNING: code 39 - INCONSISTENT_PARAMETERSWARNING: code 101 - README_FILE_MISSING
- Acquisition parameters: Link
WARNING: Incorrect RepetitionTimeWARNING: Incorrect FlipAngleWARNING: Model 'MAGNETOM Vida' not presentWARNING: Missing 'Manufacturer' key in json sidecar
- Data consistency: Link
WARNING: Missing jsonSidecarWARNING: Invalid number of columns. The schema specifies 13, but the data frame has 15
We should brainstorm on how to best fix these issues! Or, if they don't need to be fixed, we should explicitly add ignore rules for them so that they don't get thrown in the first place.
There was a problem hiding this comment.
BIDS: The WARNINGs can be ignored. (README can be fixed.) ERR should be fixed.
There was a problem hiding this comment.
Automated testing was done right before a conference, just for demo purposes, so these warnings aren't actually taken into account for acquisition parameters/data consistency.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #167.
From the issue description:
In summary: I've fixed everything I can within
validator.ymlitself, and now the checks run (and subsequently throw quite a lot of warnings, which I've escalated to errors). See https:/joshuacwnewton/data-multi-subject/actions/runs/9690897116/job/26741501317 for a sample run on my fork.But, I haven't yet done anything to actually address those errors (e.g. by setting
bids-validatorignore configs for specific warnings, fixing warnings in the repo, or updating the spine-generic scripts upstream to avoid throwing erroneous warnings/errors). That could be done here, or perhaps in a separate PR.