Skip to content

Commit cb67d18

Browse files
llucaxllucax
committed
Return an ExclusionOverlap from check_exclusion_bounds_overlap()
The function returned a `tuple[bool, bool]`, which callers matched on with one `case` per combination. mypy can't narrow tuple items across `case` clauses (python/mypy#12364), so it can't tell whether such a `match` covers every combination, and a missing case would go unnoticed. This is about to matter, as the next `frequenz-repo-config` enables mypy's `exhaustive-match` error code, which reports these two `match` statements. Return a four-valued enum instead, which mypy does check exhaustively, and which also reads better: `(True, False)` gave no hint about which of the two bounds it referred to. The two call sites in `_matryoshka.py` only cared about one combination, so they become plain `if` statements. Both matches also get an `assert_never()` case, to fail early if an unexpected value ever reaches them: type hints are not enforced at runtime. Signed-off-by: Leandro Lucarella <[email protected]>
1 parent 900fa0e commit cb67d18

2 files changed

Lines changed: 66 additions & 30 deletions

File tree

‎src/frequenz/sdk/microgrid/_power_managing/_bounds.py‎

Lines changed: 54 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,40 @@
33

44
"""Utilities for checking and clamping bounds and power values to exclusion bounds."""
55

6+
import enum
7+
from typing import assert_never
8+
69
from frequenz.quantities import Power
710

811
from ...timeseries import Bounds
912

1013

14+
# This used to be a tuple[bool, bool], but mypy can't check that a match over a tuple
15+
# covers every combination (see python/mypy#12364), so callers could leave a case out
16+
# without anyone noticing. It also reads better, as (True, False) gives no hint about
17+
# which of the two bounds it refers to.
18+
@enum.unique
19+
class ExclusionOverlap(enum.Enum):
20+
"""Which bounds of a pair fall inside an exclusion zone."""
21+
22+
NONE = enum.auto()
23+
"""Neither bound is inside the exclusion zone."""
24+
25+
LOWER = enum.auto()
26+
"""Only the lower bound is inside the exclusion zone."""
27+
28+
UPPER = enum.auto()
29+
"""Only the upper bound is inside the exclusion zone."""
30+
31+
BOTH = enum.auto()
32+
"""Both bounds are inside the exclusion zone."""
33+
34+
1135
def check_exclusion_bounds_overlap(
1236
lower_bound: Power,
1337
upper_bound: Power,
1438
exclusion_bounds: Bounds[Power] | None,
15-
) -> tuple[bool, bool]:
39+
) -> ExclusionOverlap:
1640
"""Check if the given bounds overlap with the given exclusion bounds.
1741
1842
Example:
@@ -29,31 +53,30 @@ def check_exclusion_bounds_overlap(
2953
(inside the exclusion zone)
3054
```
3155
32-
Resulting in `(False, True)` because only the upper bound is inside the
33-
exclusion zone.
56+
Resulting in `ExclusionOverlap.UPPER` because only the upper bound is inside
57+
the exclusion zone.
3458
3559
Args:
3660
lower_bound: The lower bound to check.
3761
upper_bound: The upper bound to check.
3862
exclusion_bounds: The exclusion bounds to check against.
3963
4064
Returns:
41-
A tuple containing a boolean indicating if the lower bound is bounded by the
42-
exclusion bounds, and a boolean indicating if the upper bound is bounded by
43-
the exclusion bounds.
65+
Which of the given bounds are inside the exclusion bounds.
4466
"""
4567
if exclusion_bounds is None:
46-
return False, False
47-
48-
bounded_lower = False
49-
bounded_upper = False
68+
return ExclusionOverlap.NONE
5069

51-
if exclusion_bounds.lower < lower_bound < exclusion_bounds.upper:
52-
bounded_lower = True
53-
if exclusion_bounds.lower < upper_bound < exclusion_bounds.upper:
54-
bounded_upper = True
70+
bounded_lower = exclusion_bounds.lower < lower_bound < exclusion_bounds.upper
71+
bounded_upper = exclusion_bounds.lower < upper_bound < exclusion_bounds.upper
5572

56-
return bounded_lower, bounded_upper
73+
if bounded_lower and bounded_upper:
74+
return ExclusionOverlap.BOTH
75+
if bounded_lower:
76+
return ExclusionOverlap.LOWER
77+
if bounded_upper:
78+
return ExclusionOverlap.UPPER
79+
return ExclusionOverlap.NONE
5780

5881

5982
def adjust_exclusion_bounds(
@@ -80,13 +103,16 @@ def adjust_exclusion_bounds(
80103
# And if the given bounds overlap with the exclusion bounds on one side, then clamp
81104
# the given bounds on that side.
82105
match check_exclusion_bounds_overlap(lower_bound, upper_bound, exclusion_bounds):
83-
case (True, True):
106+
case ExclusionOverlap.BOTH:
84107
return Power.zero(), Power.zero()
85-
case (False, True):
108+
case ExclusionOverlap.UPPER:
86109
return lower_bound, exclusion_bounds.lower
87-
case (True, False):
110+
case ExclusionOverlap.LOWER:
88111
return exclusion_bounds.upper, upper_bound
89-
return lower_bound, upper_bound
112+
case ExclusionOverlap.NONE:
113+
return lower_bound, upper_bound
114+
case unexpected:
115+
assert_never(unexpected)
90116

91117

92118
# Just 20 lines of code in this function, but unfortunately 8 of those are return
@@ -123,14 +149,20 @@ def clamp_to_bounds( # pylint: disable=too-many-return-statements
123149
match check_exclusion_bounds_overlap(
124150
lower_bound, upper_bound, exclusion_bounds
125151
):
126-
case (True, True):
152+
case ExclusionOverlap.BOTH:
127153
return None, None
128-
case (True, False):
154+
case ExclusionOverlap.LOWER:
129155
if value < exclusion_bounds.upper:
130156
return None, exclusion_bounds.upper
131-
case (False, True):
157+
case ExclusionOverlap.UPPER:
132158
if value > exclusion_bounds.lower:
133159
return exclusion_bounds.lower, None
160+
case ExclusionOverlap.NONE:
161+
# The bounds don't overlap the exclusion zone, so the value only needs
162+
# the generic clamping done below.
163+
pass
164+
case unexpected:
165+
assert_never(unexpected)
134166

135167
# If the given value is outside the given bounds, clamp it to the closest bound.
136168
if value < lower_bound:

‎src/frequenz/sdk/microgrid/_power_managing/_matryoshka.py‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,13 @@ def _calc_target_power(
111111
# If the bounds from the current proposal are fully within the exclusion
112112
# bounds, then don't use them to narrow the bounds further. This allows
113113
# subsequent proposals to not be blocked by the current proposal.
114-
match _bounds.check_exclusion_bounds_overlap(
115-
proposal_lower, proposal_upper, exclusion_bounds
114+
if (
115+
_bounds.check_exclusion_bounds_overlap(
116+
proposal_lower, proposal_upper, exclusion_bounds
117+
)
118+
is _bounds.ExclusionOverlap.BOTH
116119
):
117-
case (True, True):
118-
continue
120+
continue
119121
lower_bound = max(lower_bound, proposal_lower)
120122
upper_bound = min(upper_bound, proposal_upper)
121123
lower_bound, upper_bound = _bounds.adjust_exclusion_bounds(
@@ -275,11 +277,13 @@ def get_status(
275277
break
276278
proposal_lower = next_proposal.bounds.lower or lower_bound
277279
proposal_upper = next_proposal.bounds.upper or upper_bound
278-
match _bounds.check_exclusion_bounds_overlap(
279-
proposal_lower, proposal_upper, exclusion_bounds
280+
if (
281+
_bounds.check_exclusion_bounds_overlap(
282+
proposal_lower, proposal_upper, exclusion_bounds
283+
)
284+
is _bounds.ExclusionOverlap.BOTH
280285
):
281-
case (True, True):
282-
continue
286+
continue
283287
calc_lower_bound = max(lower_bound, proposal_lower)
284288
calc_upper_bound = min(upper_bound, proposal_upper)
285289
if calc_lower_bound <= calc_upper_bound:

0 commit comments

Comments
 (0)