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

Workchains: Turn exception into warning for incorrect return type in conditional predicates #265

Closed sphuber closed 1 year ago

sphuber commented 1 year ago

Fixes #262

In f47627adf0dece414c26254515f53fb7414bb3bf, the recently added check on the return type of a conditional predicate was updated slightly to only emit a warning when None would be returned instead of raising a TypeError. This was done because there quite a number of workchains that unwittingly were relying on this behavior and with the change added in 800bcf154c0ea0d4576636b95d2ad2285adec266 would break all of these.

Since then, it was discovered that the exception for None may still not be enough as it turns out that there are also quite a number of workchains that use objects that behave like booleans, but are not actually instances of the built-in bool base type. Examples, are numpy.bool as well as aiida.orm.Bool. Since here the behavior of the predicate would be as expected, breaking the existing workflow is actually not desirable.

Therefore, the requirement on the return type is relaxed slightly further to any type that implements the __bool__ method. This includes None, but also types like numpy.bool and aiida.orm.Bool which are often used in the workchain logic by AiiDA code. There might still be edge cases where users may (accidentally) return a type from a function that does implement __bool__ but whose evaluated value is not what the user intended, but that risk is preferable then forcing users to cast bool-like instances explicit to a bool.

sphuber commented 1 year ago

Alternative solution to #264

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (090e7e9) 90.83% compared to head (3a2e013) 90.82%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## support/0.21.x #265 +/- ## ================================================== - Coverage 90.83% 90.82% -0.00% ================================================== Files 21 21 Lines 2974 2971 -3 ================================================== - Hits 2701 2698 -3 Misses 273 273 ``` | [Impacted Files](https://codecov.io/gh/aiidateam/plumpy/pull/265?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam) | Coverage Δ | | |---|---|---| | [src/plumpy/workchains.py](https://codecov.io/gh/aiidateam/plumpy/pull/265?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidateam#diff-c3JjL3BsdW1weS93b3JrY2hhaW5zLnB5) | `94.35% <100.00%> (-0.04%)` | :arrow_down: | 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.

sphuber commented 1 year ago

Thanks to both for the feedback.