-
Notifications
You must be signed in to change notification settings - Fork 3
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
Expand over sessions, update config call #70
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.
This looks good to me with a couple of caveats:
- Would be good to add a test case including a session (i.e. new test dataset under
test
, add a task topyproject.toml
, and add it to thetest
workflow) - Not new to this PR, but we'll probably need to change
inputs.subj_wildcards
to a specific component.wildcards
to handle the additional wildcards we don't care about in Add additional wildcard entities #69.
You can go ahead and merge once you add the test (assuming you agree it's worth adding)
- update config calls to reflect snakebids updates (e.g. inputs['T1w'].<entity> vs config["T1w"][<entity>])
7ab41b7
to
cead754
Compare
cead754
to
f76d252
Compare
Fixed a couple of bugs with the sessions. @tkkuehn mind taking another look before I merge this in? Will also look to move towards |
5a1a1d0
to
c295a1f
Compare
Proposed changes
Resolves #63 and took advantage to update the dependencies.
Types of changes
What types of changes does your code introduce? Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you are unsure about any of the choices, don't hesitate to ask!poe quality
taskNotes
All PRs will undergo the unit testing before being reviewed. You may be requested to explain or make additional changes before the PR is accepted.