-
Notifications
You must be signed in to change notification settings - Fork 135
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
[FIX] use bids uri for qmri derivatives #339
base: master
Are you sure you want to change the base?
Conversation
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.
I think you're just missing the file://
prefix: https://bids-specification.readthedocs.io/en/latest/02-common-principles.html#source-vs-raw-vs-derived-data
Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>
I could swear we had a conversation where we dropped the |
Oh, I see. That predates the BIDS URI discussion altogether. I would still use plain relative paths. Note in Examples we use relative paths (no |
mh then we have two ways of specifying relative paths in the spec, should we clarify that? |
yes please. 😝 |
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.
I think we are safe dropping the file://
here, so my earlier recommendation was wrong (sorry).
I have opened a discussion here:
Co-authored-by: Stefan Appelhoff <stefan.appelhoff@mailbox.org>
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.
Changes here look good to me @Remi-Gau except for that one comment above. Once that is resolved, I think we can merge and mirror the change to the upstream (real) data
"sub-01/fmap/sub-01_flip-01_TB1DAM.nii.gz", | ||
"sub-01/fmap/sub-01_flip-02_TB1DAM.nii.gz" | ||
"bids:source:sub-01/sub-01/fmap/sub-01_flip-01_TB1DAM.nii.gz", | ||
"bids:source:sub-01/sub-01/fmap/sub-01_flip-02_TB1DAM.nii.gz" |
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.
I think RawSources
does not permit BIDS URIs, only dataset relative links. Also, that field is deprecated in favor of Sources
(which does permit BIDS URIs)
https://bids-specification.readthedocs.io/en/latest/glossary.html#rawsources-metadata
I think we should either just use relative links here, OR turn the RawSources
here into Sources
fixes #312
@sappelhoff does that make sense as a way to refer to source data in the derivatives using bids uri?