-
Notifications
You must be signed in to change notification settings - Fork 536
ENH: Allow afni.MaskTool to take multiple input files #2886
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
Conversation
codecov-io
commented
Feb 23, 2019
Codecov Report
@@ Coverage Diff @@ ## master #2886 +/- ## ========================================== + Coverage 67.49% 67.72% +0.22% ========================================== Files 343 343 Lines 43595 44115 +520 Branches 5424 5606 +182 ========================================== + Hits 29426 29877 +451 - Misses 13465 13529 +64 - Partials 704 709 +5
Continue to review full report at Codecov.
|
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.
Cool, thanks. If you can clean this up to specifically target the AFNI fix and fix the minor metadata issue, this can make it in for the release tomorrow.
nipype/interfaces/afni/utils.py
Outdated
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.
desc should be an argument to InputMultiPath, not File, in this case.
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.
OK, I'll correct that. Though, I simply copied a template from elsewhere in the file, e.g. same metadata issue can be found in MergeInputSpec and ZcatInputSpec class.
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.
Yeah, not everything is consistent or correct.
nipype/algorithms/modelgen.py
Outdated
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.
Can you rebase or revert the orth changes out?
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.
ah, silly, I checked out a new branch from another patch :|
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.
oops, I did a hard reset to SHA before the orth changes and then force-push to update the remote branch, but I didnt realize that one will close the pull request. I committed the AFNI changes again, but they dont appear here anymore.
Shall I simply do another pull request?
f1be111 to
98beb0a
Compare
Feel free to open a new PR, if that's easiest.
Ok, opened a new PR, at #2892
Summary
Fixes #2874.
List of changes proposed in this PR (pull-request)
Made a change to the inputs of
MaskToolclass in AFNI interface. UsedInputMultiObjectas suggested, tested it out and now it works fine with multiple input images as well as far as I can tell.Acknowledgment