bids-specification
bids-specification copied to clipboard
Downgrade absence of sub- folders to warning
I could be wrong but I think there is no strict and agreed upon requirement to require having sub- subfolders in BIDS specification. Although odd, thus warranting a warning, I think it is not strictly forbidden, and e.g. a BIDS dataset only with stimuli/ or derivatives/ or sourcedata might still be well legit BIDS dataset.
Check was added in b9ea963df27ded2c4363199c4c030c0229f2d1e2 likely just copying from prior node version of bids-validator. Asking @effigies for setting me straight ;)
The legacy validator errored on the absence of subject directories, so this was written for consistency with that.
I don't see the use-case for subject-less datasets. Calling a directory that contains opaque sourcedata/ and derivatives/ directories a BIDS dataset seems counter to the goal of BIDS, which is legible neuroimaging data. What's your goal?
With my OpenNeuro hat on, removing basic "are there any data?" checks, combined with mechanisms to shield data from validation, is just asking for turn into an arbitrary data hosting service. So this is just a note that, if this is demoted to a warning, we will need to promote it back to an error, and should probably go ahead and promote it just in case. cc @nellh
I don't see the use-case for subject-less datasets. Calling a directory that contains opaque
sourcedata/andderivatives/directories a BIDS dataset seems counter to the goal of BIDS, which is legible neuroimaging data. What's your goal?
immediate, although not mandatory if we do add dedicated DatasetType:
- https://github.com/bids-standard/bids-specification/pull/1861
and inline with that -- it could be neuro*-related (BIDS is beyond MRI for a while already) dataset of
stimuli/, or purelyphenotype/data and IMHO could still be quite legit without "neuroimaging" or per subject data. Also group level results in derivatives etc.
But also it is a matter of consistency: if there is such a rule it should be clearly described also in the specification, which I think is not the case ATM.
Indeed it might require openneuro specific configuration, but I thought you already have one anyways.
@arokem @barbarastrasser I see you gave thumbs ups on the initial post. Would either of you care to make a formal review?
For the record, I agree with the argument that this error is not reflected in the text, so we should either update the text or relax the warning. OpenNeuro will continue to make it an error.
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 82.65%. Comparing base (a7dd34a) to head (2f71c07).
:warning: Report is 293 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #1928 +/- ##
=======================================
Coverage 82.65% 82.65%
=======================================
Files 17 17
Lines 1534 1534
=======================================
Hits 1268 1268
Misses 266 266
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
@barbarastrasser I see you gave thumbs ups on the initial post. Would either of you care to make a formal review?
Yes, this is great! This way we can remove the manual downgrade in the validator config :)