-
Notifications
You must be signed in to change notification settings - Fork 296
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
DOC: Altered CLI option grouping #2944
Conversation
Thanks for opening this pull request! It looks like this is your first time contributing to fMRIPrep. 😄 "name": "Contributor, New FMRIPrep",
"affiliation": "Department of fMRI prep'ing, Open Science Made-Up University",
"orcid": "<your id>"
}, ```
Of course, if you want to opt-out this time there is no problem at all with adding your name later. You will be always welcome to add it in the future whenever you feel it should be listed. |
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.
Thank you for the contribution @Lestropie! I agree - our CLI grouping has gotten rather large and disorganized, so a restructuring is much needed. I've had a quick look and the changes look sensible to me, just left a few comments
If you would like, feel free to add yourself to the contributors file
Fixes erroneous removal of capitalisation and re-introduction of deprecated --topup-max-vols option introduced in 22dfe4b, that arose as a result of manual merging of modifications made against an older version of the code. Co-authored-by: Mathias Goncalves <goncalves.mathias@gmail.com>
Good catches @mgxd; should all be resolved. |
Back when I commenced some work on #2207 (from which I'll need to familiarise myself with #2913), I started by looking at the CLI and thinking about what the right interface would be for such a capability (see eg. #2805, #2806). I immediately found some command-line options that really felt as though they were in the wrong place. So here I've separated out the set of CLI changes that I'd generated in the course of that little experiment.
The git diff is quite unclean, so I've produced the set of options that would be moved and their corresponding old and new option groups.
--anat-only
--boilerplate-only
--md-only-boilerplate
--cifti-output
--error-on-aroma-warnings
--verbose
--me-output-echos
--medial-surface-nan
--no-submm-recon
--output-layout
--sloppy
--track-carbon
--country-code
New option groups:
Deleted option groups:
In particular, "Options for performing only a subset of the workflow" is what I was looking for in the context of #2207, where I had initially added command-line options "
--fmri_withhold
" and "--fmri_regenerate
" (the naming of such options should perhaps be discussed in #2913).Open to modification of proposed changes, or indeed addition of other changes (since this would be a good opportunity to review placement of the complete set of command-line options). For instance I'm currently looking at
[ --output-spaces, --cifti-output, --me-output-echos ]
as a candidate option group.