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

WIP nipypification of pydeface #24

Closed
wants to merge 9 commits into from

Conversation

Shotgunosine
Copy link
Collaborator

I've tried to overhaul pydeface so that the utility functions create and manipulate nipype workflows.
TODO:
deface_node and follower_node are writing directly to output directories and not writing to their working directories and then moving final outputs from the datasink to an appropriate location

Copy link
Contributor

@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.

This looks fairly reasonable at a quick glance. A few initial comments and then maybe we can look it over in greater detail later.

pydeface/__main__.py Outdated Show resolved Hide resolved
pydeface/utils.py Outdated Show resolved Hide resolved
warped_mask = list(deface_res.nodes())[6].result.otputs[0]
template_reg_mat = list(deface_res.nodes())[6].result.otputs[2]
unclean_mask = args.infile.replace('.gz', '').replace('.nii', '_pydeface_mask.nii.gz')
unclean_mat = args.infile.replace('.gz','').replace('.nii', '_pydeface.mat')
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whoops, thanks for catching that, I'd tested the utilities, but not the main interface.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Got that working, in addition to misspelling, it needed to be .outputs.out_file, but I'd love to know if there's a better way to do this.

tests/test_utils.py Outdated Show resolved Hide resolved
tests/test_utils.py Outdated Show resolved Hide resolved
@effigies
Copy link
Contributor

This will need significant rewriting, so I'm going to close this. Please reopen if you do want to try again.

@effigies effigies closed this Sep 20, 2023
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.

2 participants