-
Notifications
You must be signed in to change notification settings - Fork 295
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
[RTM] Improved EPI skullstripping #519
Conversation
(prepare_epi_opposite_wf, qwarp, [('outputnode.out_file', 'base_file')]), | ||
]) | ||
|
||
if usable_fieldmaps_matching_pe: | ||
prepare_epi_matching_wf = init_prepare_epi_wf(ants_nthreads=ants_nthreads, | ||
prepare_epi_matching_wf = init_prepare_epi_wf(ants_nthreads=omp_nthreads, |
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 is a fix of leftover variable, that should have been changed in the previous refactor.
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 reasonable. (Sorry for the delay, took a few tries to get through the whole thing.)
A few comments, but nothing substantial.
fmriprep/workflows/util.py
Outdated
outputnode = pe.Node(niu.IdentityInterface(fields=['mask_file', | ||
'skull_stripped_file', | ||
'bias_corrected_file', | ||
'reportlet']), |
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 we want to be consistent with usage, we generally call this out_report
.
fmriprep/workflows/util.py
Outdated
|
||
n4_correct = pe.Node(ants.N4BiasFieldCorrection(dimension=3), | ||
name='n4_correct') | ||
orig_hdr = pe.Node(CopyHeader(), name='orig_hdr') |
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.
Consider replacing n4_correct
and orig_hdr
with n4bias_wf
. IMO it clarifies what's going on, as well as reduces lines of code and potential for errors.
fmriprep/workflows/util.py
Outdated
@@ -8,13 +8,64 @@ | |||
from __future__ import print_function, division, absolute_import, unicode_literals | |||
|
|||
from nipype import logging | |||
|
|||
LOGGER = logging.getLogger('workflow') |
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.
We don't actually use LOGGER
anywhere in this file. Should we remove it, or keep it in case?
fmriprep/workflows/epi.py
Outdated
('t1_brain', 'fixed_image')]), | ||
(gen_ref, mask_t1w_tfm, [('out_file', 'reference_image')]), | ||
(fsl2itk_fwd, mask_t1w_tfm, [('itk_transform', 'transforms')]), | ||
(mask_t1w_tfm, outputnode, [('output_image', 'epi_mask_t1')]), | ||
(inputnode, mask_t1w_tfm, [('ref_epi_mask', 'input_image')]) |
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'd prefer if this was in a block with the other inputs to mask_t1w_tfm
.
This should be ready to go |
fmriprep/workflows/util.py
Outdated
@@ -8,12 +8,59 @@ | |||
from __future__ import print_function, division, absolute_import, unicode_literals | |||
|
|||
from nipype import logging |
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.
Sorry, one last thing: Could you drop the logging
import?
Since the workflow is built in docs, this can be a docs only commit, so we can fast track this.
Closes #515