-
Notifications
You must be signed in to change notification settings - Fork 536
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 name_template was supposed to populate the field automatically if no value was passed. With this fix, are you getting properly populated outputs?
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 nipype.
Based on the error message, we suspected the issue would come from the missing if guard. @MatthieuJoulot could you try validating the fix by updating your environment and running the pipeline again? 🙏
Hey @effigies ! Sorry for the slow response. I have checked and it doesn't work unless I specify the out_file in the parameters of the node. It does run and DWIBiasCorrect does it's thing, populates an output, but this output is not found by the next node, causing the pipeline to crash right after.
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_outputsinDWIBiasCorrectdoesn't take into account the fact thatoutput_fileis not a mandatory input, thus making any pipeline using this interface without specifying the output crash. It crashes with the following error:Screenshot 2022年09月27日 at 14 55 19