Skip to content
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: skull stripping is forced by default #2345

Merged
merged 8 commits into from
Dec 9, 2020
Merged

Conversation

jsmentch
Copy link
Contributor

@jsmentch jsmentch commented Dec 9, 2020

Changes proposed in this pull request

docs say that the deteciton heuristic is used by default but at the moment skull stripping is forced per #2318, updated docs to mention this

docs say that the heuristic is used by default but at the moment skull stripping is forced nipreps#2318, updated docs to mention this
Copy link
Collaborator

@mgxd mgxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks! Left one minor suggestion.

Would you like to add yourself as a contributor to the .zenodo.json file?

docs/workflows.rst Outdated Show resolved Hide resolved
@jsmentch
Copy link
Contributor Author

jsmentch commented Dec 9, 2020

Sure! Just did, thanks

Co-authored-by: Mathias Goncalves <goncalves.mathias@gmail.com>
Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Two small style suggestions.

docs/workflows.rst Outdated Show resolved Hide resolved
docs/workflows.rst Outdated Show resolved Hide resolved
Copy link
Member

@oesteban oesteban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this contribution, I left a few nit picks.

Comment on lines 64 to 65
T1w image is already masked. This must be explicitly requested with
``---skull-strip-t1w auto``.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
T1w image is already masked. This must be explicitly requested with
``---skull-strip-t1w auto``.
T1w image is already masked.
This may be explicitly requested with ``---skull-strip-t1w auto``.

Easier to see diffs when new sentences start with new line.

docs/workflows.rst Outdated Show resolved Hide resolved
.zenodo.json Outdated Show resolved Hide resolved
@oesteban
Copy link
Member

oesteban commented Dec 9, 2020

ha I should've refreshed the tab before commenting :D

jsmentch and others added 5 commits December 9, 2020 15:42
fix linebreaks

Co-authored-by: Chris Markiewicz <effigies@gmail.com>
fix wording

Co-authored-by: Oscar Esteban <code@oscaresteban.es>
@jsmentch
Copy link
Contributor Author

jsmentch commented Dec 9, 2020

Ok, incorporated those changes!

@effigies
Copy link
Member

effigies commented Dec 9, 2020

Great, thanks!

@effigies effigies merged commit 674124e into nipreps:master Dec 9, 2020
effigies pushed a commit to effigies/fmriprep that referenced this pull request Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants