-
Notifications
You must be signed in to change notification settings - Fork 287
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] Code revision of anatomical workflows #622
[ENH] Code revision of anatomical workflows #622
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.
Looks good. Couple minor comments. Circle issue is weird, but I don't see anything here that might have triggered it.
fmriprep/interfaces/freesurfer.py
Outdated
|
||
|
||
class FSInjectBrainExtractedOutputSpec(TraitedSpec): | ||
subjects_dir = Directory(desc='FreeSurfer SUBJECTS_HOME') |
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.
SUBJECTS_DIR
fmriprep/interfaces/freesurfer.py
Outdated
|
||
|
||
class FSInjectBrainExtractedInputSpec(BaseInterfaceInputSpec): | ||
subjects_dir = Directory(mandatory=True, desc='FreeSurfer SUBJECTS_HOME') |
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.
SUBJECTS_DIR
|
||
# https://surfer.nmr.mgh.harvard.edu/fswiki/SubmillimeterRecon | ||
mris_inflate = '-n 50' if hires else None | ||
return (t2w, hires, mris_inflate) |
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.
Is there any reason to keep these functions separate from the interfaces? The functions themselves don't make a lot of sense to use except in a pipeline (the latter especially), so there's not much need to preserve them for other uses.
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.
It is generally easier to write unit tests on these split functions, if we find the time. Other than that, I don't have a particular preference.
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.
@effigies still considering to move the code into the interfaces? Otherwise, this is ready to merge.
fmriprep/workflows/anatomical.py
Outdated
|
||
return workflow | ||
|
||
|
||
def init_gifti_surface_wf(name='gifti_surface_wf'): | ||
""" | ||
Extract surfaces from FreeSurfer derivatives folder and | ||
re-center GIFTI coordinates to fit align to native T1 space |
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.
Typo in the old docstring. Remove fit
.
Part of #225 , depends on #621
n_procs
atNode
initialization.