-
Notifications
You must be signed in to change notification settings - Fork 59
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
Handling input fields checking for Mandatory and Mutually exclusive fields #595
Conversation
for more information, see https://pre-commit.ci
Codecov ReportBase: 76.77% // Head: 76.79% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #595 +/- ##
==========================================
+ Coverage 76.77% 76.79% +0.02%
==========================================
Files 19 19
Lines 4349 4353 +4
Branches 1193 1195 +2
==========================================
+ Hits 3339 3343 +4
Misses 821 821
Partials 189 189
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. |
@ablachair - thank you so much for the fix! Could you please add your name to zenodo file. We haven't kept any specific order, but just keep Satra Ghosh as the last one. |
I will merge it now, but please add yourself to zenodo when you have a chance |
Types of changes
Summary
We identified a possible issue with Pydra task input fields checking.
Input fields can be marked as 'mandatory' as well as 'mutually exclusive'.
If
then only field_A or field_B is needed, not both.
Nipype follows this behavior, but it seems like
check_fields_input_spec()
of the Pydra engine (pydra/pydra/engine/specs.py) is checking those conditions sequentially, not concurrently.Compared with
_check_mandatory_inputs()
(nipype/interfaces/base/core.py)Note
The unit test might be misplaced. Should it go in
test_shelltask_inputspec.py
?Checklist