-
Notifications
You must be signed in to change notification settings - Fork 30
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
ENH: Make dataset description #29
Conversation
Okay, I think you're right about this. I was mostly confused about the semantics of calling this a "dataset", but I see that such a file was recommended in the original derivatives BEP. Otherwise, it very much makes sense to include such meta-data in the outputs. |
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.
Looks good
@@ -135,9 +135,7 @@ def create_workflow(opts): | |||
deriv_dir = op.join(output_dir, 'fitlins') | |||
os.makedirs(deriv_dir, exist_ok=True) |
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.
You can omit this line, since the dir is also made in write_derivative_description
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.
Does it make more sense for write_derivative_description
to assume that deriv_dir
exists?
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.
Probably, although that's what's nice about exist_ok
, is you can use it either way.
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.
If you think in the future there may be other files you might want to instantiate in the folder, you could call the function something like create_derivative_template
or something
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.
Okay. For now I'm just going to let create_workflow
do the makedirs
. Might change again in the future.
Following BEP 003:
We should start adding docker hub tags and singularity URLs as environment variables when we build images. I've put in a stub to get the MD5, which will attempt to pull the
'version'
tag from Singularity Hub.BIDS doesn't specify a field for version, which seems like an oversight, but perhaps there's another mechanism for marking it?
Long-term, it would be good to standardize the construction of this metadata in the BIDS-App spec and pybids.
Related to discussion in #28.