Skip to content

ENH: Add defacing script#318

Merged
josephmje merged 3 commits intoTIGRLab:masterfrom
josephmje:enh/deface
Nov 22, 2021
Merged

ENH: Add defacing script#318
josephmje merged 3 commits intoTIGRLab:masterfrom
josephmje:enh/deface

Conversation

@josephmje
Copy link
Copy Markdown
Contributor

@josephmje josephmje commented Nov 17, 2021 •
edited
Loading

Added a script to perform defacing.

I'm looking for some suggestions on better implementation with datman and the dashboard:

  • Should --suffix-id come from the study config?
  • I ended up saving the pydeface command to the json file. Not sure if we should overwrite the original scan. My initial preference is to keep both files. It could be good to use both to create a mask overlay QC image and then if we want, we can replace the original image after QC

@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Nov 17, 2021 •
edited
Loading

Hello @josephmje, Thank you for updating!

Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, pip install flake8 and then run flake8 datman.

Comment last updated at 2021-11-18 20:56:30 UTC

@josephmje josephmje marked this pull request as ready for review November 17, 2021 18:06
@auto-assign auto-assign bot requested review from DESm1th, jerdra and jskocic November 17, 2021 18:06
@josephmje josephmje requested review from edickie and slimnsour and removed request for jskocic November 17, 2021 18:09
DESm1th
DESm1th previously approved these changes Nov 17, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@DESm1th DESm1th left a comment

Choose a reason for hiding this comment

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

Looks good Mike! Thank you. Is there any possibility of the deface command failing silently though?

@jerdra
Copy link
Copy Markdown
Contributor

jerdra commented Nov 17, 2021

Looks good Mike! Thank you. Is there any possibility of the deface command failing silently though?

might make sense to call pydeface.utils.deface_image directly? Our inputs seem simple enough and we don't have to deal with weird cmdline arg construction or sub-process handling

@jerdra jerdra mentioned this pull request Nov 18, 2021
Open
4 tasks
@josephmje
Copy link
Copy Markdown
Contributor Author

josephmje commented Nov 18, 2021

might make sense to call pydeface.utils.deface_image directly? Our inputs seem simple enough and we don't have to deal with weird cmdline arg construction or sub-process handling

Thanks for the suggestion! Done in b0a76c6

@josephmje
- Fix CLI directory check
ef5f1af 
- Fix logic if providing a BIDS folder
- Use `deface_image` function from pydeface rather than CLI command
- Change JSON tag to `DefaceSoftware`
@jerdra jerdra mentioned this pull request Nov 19, 2021
Merged
slimnsour
slimnsour approved these changes Nov 19, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@slimnsour slimnsour left a 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! Just tested it out and it works as expected too

@josephmje josephmje merged commit a773bb7 into TIGRLab:master Nov 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jerdra jerdra jerdra approved these changes

@slimnsour slimnsour slimnsour approved these changes

@DESm1th DESm1th DESm1th left review comments

@edickie edickie Awaiting requested review from edickie

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@josephmje @pep8speaks @jerdra @DESm1th @slimnsour