Fix evaluation of conditional expressions at negative cycle points#6590
Fix evaluation of conditional expressions at negative cycle points#6590wxtim merged 5 commits intocylc:8.5.xfrom
Conversation
cylc/flow/prerequisite.py
Outdated
| fr"\b{re.escape(self.MESSAGE_TEMPLATE % t_output)}\b", | ||
| fr"{re.escape(self.MESSAGE_TEMPLATE % t_output)}\b", |
There was a problem hiding this comment.
TODO: consider the preceding comment - might need a slightly more sophisticated fix?
There was a problem hiding this comment.
(Tests all pass, but maybe the commented issue only tested the other word boundary for task name sub-strings)
There was a problem hiding this comment.
The test added in #4426 is no longer valid since universal ID was implemented.
This is needed to make the test correct
diff --git a/cylc/flow/prerequisite.py b/cylc/flow/prerequisite.py
index 717276affc..0ad3ef7eee 100644
--- a/cylc/flow/prerequisite.py
+++ b/cylc/flow/prerequisite.py
@@ -193,14 +193,13 @@ class Prerequisite:
Examples:
# GH #3644 construct conditional expression when one task name
- # is a substring of another: foo | xfoo => bar.
- # Add 'foo' to the 'satisfied' dict before 'xfoo'.
+ # is a substring of another: 11/foo | 1/foo => bar.
>>> preq = Prerequisite(1)
>>> preq[(1, 'foo', 'succeeded')] = False
- >>> preq[(1, 'xfoo', 'succeeded')] = False
- >>> preq.set_conditional_expr("1/foo succeeded|1/xfoo succeeded")
+ >>> preq[(11, 'foo', 'succeeded')] = False
+ >>> preq.set_conditional_expr("11/foo succeeded|1/foo succeeded")
>>> expr = preq.conditional_expression
>>> expr.split('|') # doctest: +NORMALIZE_WHITESPACE
- ['bool(self._satisfied[("1", "foo", "succeeded")])',
- 'bool(self._satisfied[("1", "xfoo", "succeeded")])']
+ ['bool(self._satisfied[("11", "foo", "succeeded")])',
+ 'bool(self._satisfied[("1", "foo", "succeeded")])']There was a problem hiding this comment.
I've pushed this testfix and it reveals that your fix breaks this test case
There was a problem hiding this comment.
I've fixed it, see below. The problem was for negative cycle points, the word boundary needs to be (e.g.) -/b1x not \b-1/x.
| msg = self.MESSAGE_TEMPLATE % t_output | ||
| if msg[0] == '-': | ||
| # -ve cycles: \b needs to be to the right of the `-` char. | ||
| pattern = fr"-\b{re.escape(msg[1:])}\b" | ||
| else: | ||
| pattern = fr"\b{re.escape(msg)}\b" |
There was a problem hiding this comment.
Not sure if there's a more elegant way of handling this.
|
Kicking tests |
|
Codecov patch seems to be confused, it says both 🤯 : codecov/patch — 50.00% of diff hit (target 97.00%) (Definitely 100% covered though). |
4 participants
Integer offsets
[-P2]or bigger aren't getting replaced with Python code in prerequisite conditional expressions because they generate negative cycle points when evaluated at initial cycle point 1. (There's no problem with[-P1]- that generates a cycle point of 0).Close #6588
Check List
CONTRIBUTING.mdand added my name as a Code Contributor.setup.cfg(andconda-environment.ymlif present).?.?.xbranch.