-
Notifications
You must be signed in to change notification settings - Fork 294
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: Adapt fMRIPrep to upcoming sMRIPrep API changes #2037
ENH: Adapt fMRIPrep to upcoming sMRIPrep API changes #2037
Conversation
This comment has been minimized.
This comment has been minimized.
71266cb
to
264ce92
Compare
In particular, nipreps#167 introduces a few changes that need to be considered before nipreps#1829 is merged.
264ce92
to
1236b1d
Compare
This build should be ready to pass. Comments are welcome :) |
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 to me in principle.
About #1829 feel free to start from scratch. Looks like it'll only require adding a flag to the CLI and maybe modifying an output, right?
Yup. The option and a new line in the config module for this option. |
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.
a few comments but overall looks good
omp_nthreads=config.nipype.omp_nthreads, | ||
output_dir=str(config.execution.output_dir), | ||
reportlets_dir=reportlets_dir, | ||
skull_strip_fixed_seed=config.workflow.skull_strip_fixed_seed, | ||
# skull_strip_mode='force', | ||
skull_strip_mode='force', |
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.
surely this will have a config option?
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.
yes (we are talking about that right now in #1829 )
In particular, #167 introduces a few changes that need to be considered before #1829 is merged:
skull_strip_mode
argument for the anatomical workflow (ENH: Add option to skip brain extraction smriprep#167)