Workchains: Accept but deprecate conditional predicates returning None#261
Conversation
In 800bcf1, the behavior of predicates passed to `_Conditional` instances was changed to raise if anything but a boolean was returned. This was to catch cases where users would return non-boolean values by accident, which would anyway would most likely to broken behavior of the workchain. However, quite a number of existing implementations used to do something like the following in a predicate def conditional_predicate(self): if some_condition is True: return True In the case that `some_condition` was not equal to `True`, the predicate would return `None` which would be evaluated as "falsy" and so would function as if `False` had been returned. In order to not break all implemenations using this behavior, `None` is still accepted and automatically converted to `False`. A `UserWarning` is emitted to warn that the behavior is deprecated. This is used in favor of a `DeprecationWarning` since those are not shown by default, which means the majority of users wouldn't see the deprecation warning.
sphuber
changed the title
None
|
@unkcpz I noticed that our previous change would start breaking a lot of workchains out there. Fortunately we haven't release it in |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## release/0.21.5 #261 +/- ##
==================================================
+ Coverage 90.81% 90.83% +0.02%
==================================================
Files 21 21
Lines 2970 2974 +4
==================================================
+ Hits 2697 2701 +4
Misses 273 273
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 in Codecov by Sentry. |
|
Thanks @sphuber, can you point out which workchain return But you are correct if this breaks some workflow at the moment it is not good and the warning should be raised. I agree with this change but the phrase "please return |
The workchain that triggered it for me is a private internal one. But I sketched out the relevant logic in the commit message and that does seem something that users could easily have used. Since Python does not enforce typing, I think it would be easy for users to return
I agree that we cannot know for sure, but I think as an
Why not? Before the change, |
Yes, you are right, I didn't notice None is treated as False. It makes all sense to me now. |
#261) In 800bcf1, the behavior of predicates passed to `_Conditional` instances was changed to raise if anything but a boolean was returned. This was to catch cases where users would return non-boolean values by accident, which would anyway would most likely to broken behavior of the workchain. However, quite a number of existing implementations used to do something like the following in a predicate def conditional_predicate(self): if some_condition is True: return True In the case that `some_condition` was not equal to `True`, the predicate would return `None` which would be evaluated as "falsy" and so would function as if `False` had been returned. In order to not break all implemenations using this behavior, `None` is still accepted and automatically converted to `False`. A `UserWarning` is emitted to warn that the behavior is deprecated. This is used in favor of a `DeprecationWarning` since those are not shown by default, which means the majority of users wouldn't see the deprecation warning. Cherry-pick: f47627a
aiidateam#261) In 800bcf1, the behavior of predicates passed to `_Conditional` instances was changed to raise if anything but a boolean was returned. This was to catch cases where users would return non-boolean values by accident, which would anyway would most likely to broken behavior of the workchain. However, quite a number of existing implementations used to do something like the following in a predicate def conditional_predicate(self): if some_condition is True: return True In the case that `some_condition` was not equal to `True`, the predicate would return `None` which would be evaluated as "falsy" and so would function as if `False` had been returned. In order to not break all implemenations using this behavior, `None` is still accepted and automatically converted to `False`. A `UserWarning` is emitted to warn that the behavior is deprecated. This is used in favor of a `DeprecationWarning` since those are not shown by default, which means the majority of users wouldn't see the deprecation warning. Cherry-pick: f47627a
aiidateam#261) In 800bcf1, the behavior of predicates passed to `_Conditional` instances was changed to raise if anything but a boolean was returned. This was to catch cases where users would return non-boolean values by accident, which would anyway would most likely to broken behavior of the workchain. However, quite a number of existing implementations used to do something like the following in a predicate def conditional_predicate(self): if some_condition is True: return True In the case that `some_condition` was not equal to `True`, the predicate would return `None` which would be evaluated as "falsy" and so would function as if `False` had been returned. In order to not break all implemenations using this behavior, `None` is still accepted and automatically converted to `False`. A `UserWarning` is emitted to warn that the behavior is deprecated. This is used in favor of a `DeprecationWarning` since those are not shown by default, which means the majority of users wouldn't see the deprecation warning. Cherry-pick: f47627a
#261) In 800bcf1, the behavior of predicates passed to `_Conditional` instances was changed to raise if anything but a boolean was returned. This was to catch cases where users would return non-boolean values by accident, which would anyway would most likely to broken behavior of the workchain. However, quite a number of existing implementations used to do something like the following in a predicate def conditional_predicate(self): if some_condition is True: return True In the case that `some_condition` was not equal to `True`, the predicate would return `None` which would be evaluated as "falsy" and so would function as if `False` had been returned. In order to not break all implemenations using this behavior, `None` is still accepted and automatically converted to `False`. A `UserWarning` is emitted to warn that the behavior is deprecated. This is used in favor of a `DeprecationWarning` since those are not shown by default, which means the majority of users wouldn't see the deprecation warning. Cherry-pick: f47627a
#261) In 800bcf1, the behavior of predicates passed to `_Conditional` instances was changed to raise if anything but a boolean was returned. This was to catch cases where users would return non-boolean values by accident, which would anyway would most likely to broken behavior of the workchain. However, quite a number of existing implementations used to do something like the following in a predicate def conditional_predicate(self): if some_condition is True: return True In the case that `some_condition` was not equal to `True`, the predicate would return `None` which would be evaluated as "falsy" and so would function as if `False` had been returned. In order to not break all implemenations using this behavior, `None` is still accepted and automatically converted to `False`. A `UserWarning` is emitted to warn that the behavior is deprecated. This is used in favor of a `DeprecationWarning` since those are not shown by default, which means the majority of users wouldn't see the deprecation warning. Cherry-pick: f47627a
2 participants
In 800bcf1, the behavior of predicates passed to
_Conditionalinstances was changed to raise if anything but a boolean was returned. This was to catch cases where users would return non-boolean values by accident, which would anyway would most likely to broken behavior of the workchain.However, quite a number of existing implementations used to do something like the following in a predicate
In the case that
some_conditionwas not equal toTrue, the predicate would returnNonewhich would be evaluated as "falsy" and so would function as ifFalsehad been returned.In order to not break all implemenations using this behavior,
Noneis still accepted and automatically converted toFalse. AUserWarningis emitted to warn that the behavior is deprecated. This is used in favor of aDeprecationWarningsince those are not shown by default, which means the majority of users wouldn't see the deprecation warning.