-
Notifications
You must be signed in to change notification settings - Fork 533
FIX: Improve output handling in DWIDenoise and DWIBiasCorrect #2978
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
@oesteban I was wondering if I could get your feedback on this fix? Thank you! |
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.
Overall looks good. Some comments.
Also:
remove
use_default=True
so that values can be changed
usedefault=True
does not prevent overriding.
Codecov Report
@@ Coverage Diff @@
## master #2978 +/- ##
==========================================
+ Coverage 67.61% 67.62% +0.01%
==========================================
Files 344 344
Lines 43798 43782 -16
Branches 5471 5469 -2
==========================================
- Hits 29612 29606 -6
+ Misses 13475 13465 -10
Partials 711 711
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.
It's usedefault
, not use_default
. And one other comment.
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.
LGTM.
Summary
Fixes #2975 .
List of changes proposed in this PR (pull-request)
name_template
to auto-generate noise file_list_outputs()
methoduse_default=True
so that values can be changedgrad_file
andgrad_fsl
fromMRTrix3Base
spec and add xor since arguments are mutually exclusivemin_version=5.0.10
since the Mrtrix3 version is being checked, not FSL_list_outputs()
methodAcknowledgment