Skip to content

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

Merged
merged 3 commits into from
Oct 17, 2022

Conversation

MatthieuJoulot
Copy link
Contributor

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. It crashes with the following error:

Screenshot 2022-09-27 at 14 55 19

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
Copy link

codecov bot commented Sep 28, 2022

Codecov Report

Base: 65.28% // Head: 65.28% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (ddc81d3) compared to base (cf264ac).
Patch coverage: 0.00% of modified lines in pull request are covered.

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              
Flag Coverage Δ
unittests 65.03% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
nipype/interfaces/mrtrix3/preprocess.py 79.86% <0.00%> (-0.56%) ⬇️

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ghisvail
Copy link
Contributor

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.

ghisvail added a commit to ghisvail/clinica that referenced this pull request Sep 29, 2022
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>
ghisvail added a commit to aramis-lab/clinica that referenced this pull request Sep 29, 2022
Version 1.8.3 introduced a regression to the DWI bias correction interface.

See nipy/nipype#3523
@effigies effigies changed the title Fix non mandatory output issue in DWIBiasCorrect FIX: Check for non-mandatory output in DWIBiasCorrect Sep 29, 2022
@effigies
Copy link
Member

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?

@ghisvail
Copy link
Contributor

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? 🙏

@MatthieuJoulot
Copy link
Contributor Author

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.

@effigies
Copy link
Member

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.

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.

3 participants