[rfc]changing Task initialization syntax closes #295

[djarecka]

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [x] Breaking change (fix or feature that would cause existing functionality to change)

Summary

changing syntax for adding input fields during initialization (see discussion in #295)

• all inputs to the python function has to be included in inputs dictionary (and not as kwargs)
• for ShellCommandTask and ContainerTask everything but the default input fields (executable, args, image, bindings) also has to be provided in the inputs dictionary

Checklist

• [x] All tests passing
• [x] I have added tests to cover my changes
• [ ] I have updated documentation (if necessary)
• [x] My code follows the code style of this project (we are using black: you can pip install pre-commit, run pre-commit install in the pydra directory and black will be run automatically with each commit)

Acknowledgment

• [x] I acknowledge that this contribution will be available under the Apache 2 license.

[codecov[bot]]

Codecov Report

Merging #305 into master will increase coverage by 3.23%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #305      +/-   ##
==========================================
+ Coverage   82.87%   86.11%   +3.23%     
==========================================
  Files          20       20              
  Lines        3253     3262       +9     
  Branches      861      864       +3     
==========================================
+ Hits         2696     2809     +113     
+ Misses        404      291     -113     
- Partials      153      162       +9     

Flag	Coverage Δ
#unittests	86.11% <100.00%> (+3.23%)	:arrow_up:

Impacted Files	Coverage Δ
pydra/engine/core.py	89.79% <ø> (ø)
pydra/engine/specs.py	90.90% <100.00%> (+0.34%)	:arrow_up:
pydra/engine/task.py	81.88% <100.00%> (+0.24%)	:arrow_up:
pydra/utils/profiler.py	38.16% <0.00%> (-5.35%)	:arrow_down:
pydra/conftest.py	77.77% <0.00%> (ø)
pydra/engine/submitter.py	90.51% <0.00%> (+1.45%)	:arrow_up:
pydra/engine/helpers.py	85.39% <0.00%> (+6.74%)	:arrow_up:
pydra/engine/workers.py	78.60% <0.00%> (+38.30%)	:arrow_up:
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 54cf1b8...12201b9. Read the comment docs.

[satra]

this is an api breaking change and let's take a decision on this and update the few repos that we have that use pydra

[djarecka]

@satra - so you don't want to have it today/tomorrow release.

[satra]

i mean this is as good a time as any, but perhaps we should ask a few people. because once changed, we would unlikely change it back.

[djarecka]

@satra - you're right, should give more time. I wasn't planning to change tutorial before accepting this (pretty sure that won't discover anything new after changes in tests), but let me know if you want me to do it

[satra]

i think we can have people look at this and if they agree we make another release soon after, but let's leave this out of the immediate release.

[djarecka]

@satra - i have to say that I like out current syntax, after all these changes I started thinking that perhaps it could be kept as an option, i.e. if inputs is used, we expect everything to be in the inputs dictionary, but if inputs is not specified than we take kwargs.

[satra]

@effigies and @oesteban - would love your input on this. i agree with dorota's last comment. that it would make it much easier in most cases to take kwargs as input fields if there is no clash. but allow for inputs keyword if a user choses.

@djarecka - since the issue here is about clashes with potential attributes of the classes. perhaps we can check if the input spec has a field that clashes and in those cases force people to use inputs.

[effigies]

I think that seems fine. I'm very likely to develop patterns where I can be very consistent, whether that's avoiding parameters that clash or always using inputs, but we don't have to make it impossible to do otherwise.

[djarecka]

@satra - but perhaps we should not allow for mixing, i.e. some inputs in inputs some in the kwargs

[satra]

we should not allow for mixing

yes no mixing.

[effigies]

We really let this one fall by the wayside. Going to need to be redone post-#662.