Skip to content

Conversation

@yarikoptic
Copy link
Collaborator

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 b9ea963 likely just copying from prior node version of bids-validator. Asking @effigies for setting me straight ;)

@effigies
Copy link
Collaborator

effigies commented Oct 3, 2024

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

@yarikoptic
Copy link
Collaborator Author

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?

immediate, although not mandatory if we do add dedicated DatasetType:

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.

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.
@effigies effigies force-pushed the enh-warning-no-sub branch from d3744b1 to 2f71c07 Compare April 25, 2025 13:10
@effigies
Copy link
Collaborator

@arokem @barbarastrasser I see you gave thumbs ups on the initial post. Would either of you care to make a formal review?

@effigies
Copy link
Collaborator

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
Copy link

codecov bot commented Apr 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.65%. Comparing base (a7dd34a) to head (2f71c07).
⚠️ 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           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@barbarastrasser
Copy link

@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 :)

@effigies effigies merged commit 67467c7 into bids-standard:master May 15, 2025
27 checks passed
@effigies effigies added schema Issues related to the YAML schema representation of the specification. Patch version release. exclude-from-changelog This item will not feature in the automatically generated changelog labels Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

exclude-from-changelog This item will not feature in the automatically generated changelog schema Issues related to the YAML schema representation of the specification. Patch version release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants