Add BidsValidator Plugin #291

kaitj · 2023-05-09T12:58:58Z

Proposed changes

A second attempt at implementing bids validation (see #222). This time, implementing as a snakebids plugin (resuing some of the code from the previous attempt). The plugin tries to perform validation using the bids-validator unless skipped by the user (--skip_bids_validation). If the validation fails, an InvalidBidsError is thrown, otherwise validation is performed by default using pybids with a warning informing the user.

As noted in #222:

Drawing some inspiration from fmriprep, a temporary config file is generated for the system call in (_validate_bids_dir) which includes files to ignore during this validation. Was also wondering if we want to limit the validation to the subject data themselves rather than a full dataset (e.g. ignore participants.tsv, dataset_description.json)?

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you are unsure about any of the choices, don't hesitate to ask!

Changes have been tested to ensure that fix is effective or that a feature works.
Changes pass the unit tests
I have included necessary documentation or comments (as necessary)
Any dependent changes have been merged and published

Notes

All PRs will undergo the unit testing before being reviewed. You may be requested to explain or make additional changes before the PR is accepted.

snakebids/core/input_generation.py

snakebids/plugins/validate.py

snakebids/tests/test_validate_plugin.py

docs/running_snakebids/overview.md

snakebids/core/input_generation.py

tkkuehn

Looking good, just a few typos and one suggestion for a potential enhancement to the plugin

docs/bids_app/plugins.md

docs/running_snakebids/overview.md

snakebids/plugins/validator.py

snakebids/project_template/{{cookiecutter.app_name}}/config/snakebids.yml

snakebids/tests/test_validate_plugin.py

kaitj · 2023-05-12T13:07:14Z

The one check that is failing seems to have occurred when there was some GitHub downtime the other day. I’ve reran this a couple of times since and that error is still there.

tkkuehn

Yeah it is not at all clear to me what's going on with that workflow failure, I think we can probably just merge and address it in a separate PR if it sticks around

kaitj · 2023-05-19T18:34:45Z

Looks like making a new push fixed that workflow failure.

pvandyken · 2023-05-19T22:42:35Z

@kaitj, are you still working on this one, or am I good to take another look?

kaitj · 2023-05-19T22:50:08Z

@kaitj, are you still working on this one, or am I good to take another look?

Should be good for another look.

pvandyken

Apologies again for the delay, I left some comments

docs/bids_app/plugins.md

snakebids/project_template/{{cookiecutter.app_name}}/workflow/Snakefile

snakebids/tests/helpers.py

snakebids/tests/test_generate_inputs.py

snakebids/tests/test_validate_plugin.py

pvandyken

Sorry for another round of comments, I think I just reviewed this too late last night

docs/running_snakebids/overview.md

snakebids/core/input_generation.py

snakebids/plugins/validator.py

snakebids/project_template/{{cookiecutter.app_name}}/{{cookiecutter.app_name}}/run.py

snakebids/plugins/validator.py

pvandyken

Left one comment RE documentation, but I'd be okay with deferring to a future issue if you'd rather not do it here, I'll leave it up to you

snakebids/plugins/validator.py

snakebids/project_template/{{cookiecutter.app_name}}/workflow/Snakefile

- add validation via node.js Bids-Validator - add information about use of validation and plugins to the documentation - ignore validation on existing test cases - create test cases for BidsValidator plugin

- make validate=False default behaviour - fix bug with subprocess call for bids-validator

- use a namespaced config key (e.g. plugins.validator.param) - update documentation for namespaced key example

- validate with pybids if bids-validation is to be performed (regardless of ability to use bids-validator

- update overview to include link to plugins page for using / creating plugins - revamp plugins doc to split use and creation of plugins - using plugins demonstrates how to add a plugin to the snakebids app with distributed validator as example - creating plugins updated from function to callable class - highlights recommended practices for inheriting from SnakebidsPluginError for errors and namespaced keys for config - fix doc and cookiecutter typos / grammar - rename validate.py -> validator.py for consistency

- remove app assignment (self.app -> app) - remove logger from validator testing

- clarify '...callable class...' -> '...function or callable class...' - fix spelling

- remove mention of uncommenting lines - rephrased "Enable by default..." to "Boilerplate starts with the validator plugin enabled..." in the overview doc - focused docstring of validation using pybids in `generate_inputs()` - associte pybids validate logic to plugins.validator.skip (--skip-bids-validation), which will make it simpler for downstream devs - left plugins.validator.success in as it can still be used for more complex logic if desired - instantiate validator plugin in boilerplate - moved docstring under `__call__` - replaced `__init__` with attr - remove logic comments in Snakefile - replace individual #noqa: PLR0913 calls to ignore under ruff configuration

kaitj added the enhancement New feature or request label May 9, 2023

github-actions bot requested review from akhanf, pvandyken and tkkuehn May 9, 2023 12:59

kaitj linked an issue May 9, 2023 that may be closed by this pull request

integrate bids validator by default into snake bids #222

Closed

github-actions bot assigned kaitj May 9, 2023

kaitj mentioned this pull request May 9, 2023

BIDS validation #259

Closed

7 tasks

pvandyken requested changes May 9, 2023

View reviewed changes

kaitj force-pushed the bids-plugin branch from 316b83a to d4660e9 Compare May 10, 2023 10:22

tkkuehn requested changes May 10, 2023

View reviewed changes

kaitj force-pushed the bids-plugin branch from f191342 to 3d85fb4 Compare May 10, 2023 18:52

tkkuehn approved these changes May 12, 2023

View reviewed changes

kaitj requested a review from pvandyken May 19, 2023 16:13

kaitj force-pushed the bids-plugin branch from 69a0398 to 618b763 Compare May 19, 2023 18:26

kaitj force-pushed the bids-plugin branch from 618b763 to 97891e3 Compare May 23, 2023 21:15

pvandyken requested changes Jun 7, 2023

View reviewed changes

kaitj requested a review from pvandyken June 7, 2023 15:58

pvandyken requested changes Jun 7, 2023

View reviewed changes

kaitj force-pushed the bids-plugin branch 3 times, most recently from 2bde868 to 8ba03a7 Compare June 8, 2023 19:58

kaitj requested a review from pvandyken June 8, 2023 20:02

pvandyken approved these changes Jun 22, 2023

View reviewed changes

snakebids/plugins/validator.py Outdated Show resolved Hide resolved

snakebids/project_template/{{cookiecutter.app_name}}/workflow/Snakefile Outdated Show resolved Hide resolved

kaitj added 3 commits June 22, 2023 13:26

create BidsValidator plugin

52c1bae

- add validation via node.js Bids-Validator - add information about use of validation and plugins to the documentation - ignore validation on existing test cases - create test cases for BidsValidator plugin

fix subprocess call for bids-validator

63a01b5

- make validate=False default behaviour - fix bug with subprocess call for bids-validator

use namespaced config keys for plugin

ff2e954

- use a namespaced config key (e.g. plugins.validator.param) - update documentation for namespaced key example

kaitj added 9 commits June 22, 2023 13:26

make _gen_bids_layout kw-only, update tests

7728080

add a SnakebidsPluginError to exceptions

04de7a8

update pybids validation conditions

39bd401

- validate with pybids if bids-validation is to be performed (regardless of ability to use bids-validator

rm unnecessary vars

60a7ba7

- remove app assignment (self.app -> app) - remove logger from validator testing

make catching InvalidBidsError configurable

c941473

updates to documentation

715e94d

- clarify '...callable class...' -> '...function or callable class...' - fix spelling

update validate with config.get in generate_inputs

622f5f7

rm unneeded validate=False in tests

8c3d707

kaitj force-pushed the bids-plugin branch 2 times, most recently from 61edbbc to d733c9f Compare June 22, 2023 17:37

kaitj force-pushed the bids-plugin branch from d733c9f to 93532f3 Compare June 22, 2023 18:19

kaitj mentioned this pull request Jun 22, 2023

Plugins page in docs #311

Closed

kaitj merged commit e3f6da8 into khanlab:main Jun 23, 2023

kaitj deleted the bids-plugin branch June 23, 2023 12:02

kaitj changed the title ~~Bids-validator plugin~~ Add BidsValidator Plugin Jun 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BidsValidator Plugin #291

Add BidsValidator Plugin #291

kaitj commented May 9, 2023

tkkuehn left a comment

kaitj commented May 12, 2023 •

edited

Loading

tkkuehn left a comment

kaitj commented May 19, 2023

pvandyken commented May 19, 2023

kaitj commented May 19, 2023

pvandyken left a comment

pvandyken left a comment

pvandyken left a comment

Add BidsValidator Plugin #291

Add BidsValidator Plugin #291

Conversation

kaitj commented May 9, 2023

Proposed changes

Checklist

Notes

tkkuehn left a comment

Choose a reason for hiding this comment

kaitj commented May 12, 2023 • edited Loading

tkkuehn left a comment

Choose a reason for hiding this comment

kaitj commented May 19, 2023

pvandyken commented May 19, 2023

kaitj commented May 19, 2023

pvandyken left a comment

Choose a reason for hiding this comment

pvandyken left a comment

Choose a reason for hiding this comment

pvandyken left a comment

Choose a reason for hiding this comment

kaitj commented May 12, 2023 •

edited

Loading