-
Notifications
You must be signed in to change notification settings - Fork 532
FIX: Check for non-mandatory output in DWIBiasCorrect #3523
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
The function `_list_outputs` in `DWIBiasCorrect` doesn't take into account the fact that `output_file` is not a mandatory input, thus making any pipeline using this interface without specifying the output crash.
Codecov ReportBase: 65.28% // Head: 65.28% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #3523 +/- ##
==========================================
- Coverage 65.28% 65.28% -0.01%
==========================================
Files 309 309
Lines 40872 40873 +1
Branches 5380 5381 +1
==========================================
Hits 26684 26684
- Misses 13110 13111 +1
Partials 1078 1078
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This is a regression introduced in version 1.8.3, which affects the DWI pipelines of Clinica. We will need to put an upper bound for the version of nipype as a short term solution, but it would be very nice to have this fix land in the next release. |
Version 1.8.3 introduced a regression to the DWI bias correction interface. See nipy/nipype#3523
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
Version 1.8.3 introduced a regression to the DWI bias correction interface. See nipy/nipype#3523
I'm actually a little confused by this. I thought |
What we know is that our DWI pipeline used to work before the changes introduced in version 1.8.3. Our CI had version 1.8.2 locked in the test dependencies so we were only able to catch the regression later, when we deployed a brand new environment which included the latest version of Based on the error message, we suspected the issue would come from the missing |
Hey @effigies ! Sorry for the slow response. I have checked and it doesn't work unless I specify the |
Hmm. That seems like a problem, but one that I'm not prepared to debug right now. I think it's sensible to get this fix in in the short term. |
The function
_list_outputs
inDWIBiasCorrect
doesn't take into account the fact thatoutput_file
is not a mandatory input, thus making any pipeline using this interface without specifying the output crash. It crashes with the following error: