-
Notifications
You must be signed in to change notification settings - Fork 92
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
[MRG] Adds Sources for scans.tsv files #890
Conversation
@sappelhoff does the bids-validator prevent a Sources key value in the sidecar JSONs? |
I would recommend with PRs until the BIDS side is done, else you might end up investing too much effort |
Co-authored-by: Richard Höchenberger <[email protected]>
Now CIs should fully pass with the latest addition to only write lower-case EDF |
Codecov Report
@@ Coverage Diff @@
## main #890 +/- ##
==========================================
+ Coverage 94.73% 94.78% +0.04%
==========================================
Files 23 23
Lines 3460 3487 +27
==========================================
+ Hits 3278 3305 +27
Misses 182 182
Continue to review full report at Codecov.
|
Is there a specific reason why the column name is plural ("sources") and not singular? |
Good point. I was keeping in line with |
Otherwise LGTM! |
Co-authored-by: Richard Höchenberger <[email protected]>
@sappelhoff lmk if this good to merge? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two tiny nitpicks
Co-authored-by: Richard Höchenberger <[email protected]>
Thank you, @adam2392!! |
Thanks adam :) |
PR Description
Fixes: #887
Waiting on: bids-standard/bids-specification#906Merge checklist
Maintainer, please confirm the following before merging: