aiidateam / plumpy

A python workflows library that supports writing Processes with a well defined set of inputs and outputs that can be strung together.
https://plumpy.readthedocs.io
Other
8 stars 17 forks source link

`PortNamespace.get_port`: Only create if `create_dynamically` is `True` #268

Closed sphuber closed 1 year ago

sphuber commented 1 year ago

Fixes #267

In https://github.com/aiidateam/plumpy/commit/4c29f4459c8eb8a8263049ac338189c604702e4e, the get_port method was updated to automatically create a port if it didn't exist and the namespace is dynamic instead of raising. But this had unwanted knock-on consequences (see previous commit for details).

Here, the change in behavior is only triggered when the new argument create_dynamically is explicitly set to True. This means that by default the old behavior is maintained of a ValueError being raised if the requested port doesn't exist. But now Process.out can override the default to True to automatically support nested namespaces in a dynamic output namespace.

In the first commit, two tests are added. One for the desired behavior of Process.out and one as a regression test for the bug that was introduced with respect to the validation of excluded exposed ports.

sphuber commented 1 year ago

@danielhollas I think this should fix the problem of your workchain while maintaining the functionality that the commit in question tried to introduce. I still think that the use-case it is trying to address makes sense, but please let me know if you think it doesn't and the user should be forced to be more explicit.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (485b196) 90.82% compared to head (9d45c41) 90.82%.

:exclamation: Current head 9d45c41 differs from pull request most recent head 89eef4a. Consider uploading reports for the commit 89eef4a to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## support/0.21.x #268 +/- ## =============================================== Coverage 90.82% 90.82% =============================================== Files 21 21 Lines 2973 2973 =============================================== Hits 2700 2700 Misses 273 273 ``` | [Impacted Files](https://codecov.io/gh/aiidateam/plumpy/pull/268?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam) | Coverage Δ | | |---|---|---| | [src/plumpy/processes.py](https://codecov.io/gh/aiidateam/plumpy/pull/268?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam#diff-c3JjL3BsdW1weS9wcm9jZXNzZXMucHk=) | `92.46% <ø> (ø)` | | | [src/plumpy/ports.py](https://codecov.io/gh/aiidateam/plumpy/pull/268?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam#diff-c3JjL3BsdW1weS9wb3J0cy5weQ==) | `93.42% <100.00%> (ø)` | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

danielhollas commented 1 year ago

@sphuber thanks! I am happy to report that my workchain is happy with version 0.21.7. :-) I've opened a PR on plumpy-feedstock, I need a release on conda to be able to release a new aiidalab docker image.