-
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
Labelmerge #30
Labelmerge #30
Conversation
- Added tpl and updated names for MNI152NLin6Asym - Renamed MNI2009b -> MNI152NLin2009bAsym - Removed unnecessary files - Update config
- Update config to include tsv for subcortical labels
- Update freesurfer tsv to match bids spec - Update file name of Freesurfer dseg tsv
- Add inputs to rule all for copying freesurfer and zona tsv files to appropriate location
- Still need to fix piping of workflow, update entities, and filenames
- Adds in rule for labelmerge (needs updating with container and proper command) - Update resources with correct names and paths - Add tsv to rule all for copying from resource to bids folders for labelmerge - Simplify rule all
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, just wanna make sure the double check comment isn't blocking before I merge this.
@@ -140,6 +140,7 @@ rule dwi2response: | |||
|
|||
rule responsemean: | |||
"""Compute average response function""" | |||
# DOUBLE CHECK |
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.
What are we double checking here?
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.
Ahh, this was a note to my self, I removed it in one of the other PRs. Just wanted to check the conditional was written correctly by comparing with an existing rule written in another workflow.
Tests are failing because the |
Proposed changes
Update (Nov 30): Changed the base branch to
dry-run
as the tests were helpful to making sure that the changes don't break the workflow.zona_bb_subcortex.smk
with rule for labelmergesingularity run
can be called. Otherwise snakemake just tries to run the rule using the container as a dependency.One note is that this PR assumes that khanlab/labelmerge#22 is good to proceed.
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.