From 7999dfe6adf6277232ecd00ec8122ad3c686aef5 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 2 Jul 2026 15:30:14 +0200 Subject: [PATCH 1/3] Remove `None` from `Interval` type parameter `Interval`'s type parameter no longer includes `None`. The exported type variable `LessThanComparableOrNoneT` (bound to `LessThanComparable | None`) was deprecated and replaced with `LessThanComparableT` (bound to `LessThanComparable`). `None` is still accepted as a value for `start` / `end` to indicate an unbounded side, but it is treated purely as bound metadata rather than a value in the interval's comparable space. Note that explicitly using `Interval[int | None]` still works, as `(int | None) | None` is equivalent to `int | None`, even when it is no longer needed nor recommended. Old code will mostly work, but there is one soft breaking change: `None in some_interval` and `None in some_interval_set` is now a type-check error at call sites, matching the design intent that `None` is a bound marker and never a member value. Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 10 ++++++- src/frequenz/core/math.py | 58 ++++++++++++++++--------------------- tests/math/test_interval.py | 4 +-- 3 files changed, 35 insertions(+), 37 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 67f33e6..5261a49 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -6,7 +6,15 @@ ## Upgrading - +- [`Interval`][frequenz.core.math.Interval]'s type parameter no longer includes `None`. The exported type variable `LessThanComparableOrNoneT` (bound to `LessThanComparable | None`) was deprecated and replaced with `LessThanComparableT` (bound to `LessThanComparable`). `None` is still accepted as a value for `start` / `end` to indicate an unbounded side, but it is treated purely as bound metadata rather than a value in the interval's comparable space. + + Migration: + + - `Interval[int | None]` → `Interval[int]` + - `Interval[LessThanComparable | None]` → `Interval[LessThanComparable]` + - `LessThanComparableOrNoneT` (importable name) → `LessThanComparableT` + + Note that explicitly using `Interval[int | None]` still works, as `(int | None) | None` is equivalent to `int | None`, even when it is no longer needed nor recommended. Old code will mostly work, but there is one soft breaking change: `None in some_interval` and `None in some_interval_set` is now a type-check error at call sites, matching the design intent that `None` is a bound marker and never a member value. ## New Features diff --git a/src/frequenz/core/math.py b/src/frequenz/core/math.py index 2fca7a9..2d535f3 100644 --- a/src/frequenz/core/math.py +++ b/src/frequenz/core/math.py @@ -5,7 +5,7 @@ import math from dataclasses import dataclass -from typing import Generic, Protocol, Self, TypeVar, cast +from typing import Generic, Protocol, Self, TypeVar def is_close_to_zero(value: float, abs_tol: float = 1e-9) -> bool: @@ -34,14 +34,23 @@ def __lt__(self, other: Self, /) -> bool: """Return whether self is less than other.""" +LessThanComparableT = TypeVar("LessThanComparableT", bound=LessThanComparable) +"""Type variable for a value that is [`LessThanComparable`][..LessThanComparable].""" + + LessThanComparableOrNoneT = TypeVar( "LessThanComparableOrNoneT", bound=LessThanComparable | None ) -"""Type variable for a value that a `LessThanComparable` or `None`.""" +"""Type variable for a value that is [`LessThanComparable`][..LessThanComparable] or `None`. + +Warning: Deprecated + This type variable is deprecated and it will be removed in a future version. Use + [`LessThanComparableT`][..LessThanComparableT] instead. +""" @dataclass(frozen=True, repr=False) -class Interval(Generic[LessThanComparableOrNoneT]): +class Interval(Generic[LessThanComparableT]): """An interval to test if a value is within its limits. The [`.start`][.start] and [`.end`][.end] are inclusive, meaning that the @@ -49,7 +58,8 @@ class Interval(Generic[LessThanComparableOrNoneT]): checking if a value is contained by the interval. If the [`.start`][.start] or [`.end`][.end] is `None`, it means that the interval - is unbounded in that direction. + is unbounded in that direction. `None` is used purely as a bound marker; it is + never a value in the interval. If [`.start`][.start] is bigger than [`.end`][.end], a `ValueError` is raised. @@ -57,25 +67,23 @@ class Interval(Generic[LessThanComparableOrNoneT]): the `__lt__` method to be able to compare values. """ - start: LessThanComparableOrNoneT - """The start of the interval.""" + start: LessThanComparableT | None + """The start of the interval, or `None` to indicate no lower bound (-∞).""" - end: LessThanComparableOrNoneT - """The end of the interval.""" + end: LessThanComparableT | None + """The end of the interval, or `None` to indicate no upper bound (+∞).""" def __post_init__(self) -> None: """Check if the start is less than or equal to the end.""" if self.start is None or self.end is None: return - start = cast(LessThanComparable, self.start) - end = cast(LessThanComparable, self.end) - if start > end: + if self.start > self.end: raise ValueError( f"The start ({self.start}) can't be bigger than end ({self.end})" ) - def __contains__(self, item: LessThanComparableOrNoneT) -> bool: - """Check if the value is within the range of the container. + def __contains__(self, item: LessThanComparableT) -> bool: + """Check if the value is within the range of the interval. Args: item: The value to check. @@ -83,27 +91,11 @@ def __contains__(self, item: LessThanComparableOrNoneT) -> bool: Returns: True if value is within the range, otherwise False. """ - if item is None: + if self.start is not None and item < self.start: + return False + if self.end is not None and item > self.end: return False - casted_item = cast(LessThanComparable, item) - - if self.start is None and self.end is None: - return True - if self.start is None: - start = cast(LessThanComparable, self.end) - return not casted_item > start - if self.end is None: - return not self.start > item - # mypy seems to get confused here, not being able to narrow start and end to - # just LessThanComparable, complaining with: - # error: Unsupported left operand type for <= (some union) - # But we know if they are not None, they should be LessThanComparable, and - # actually mypy is being able to figure it out in the lines above, just not in - # this one, so it should be safe to cast. - return not ( - casted_item < cast(LessThanComparable, self.start) - or casted_item > cast(LessThanComparable, self.end) - ) + return True def __repr__(self) -> str: """Return a string representation of this instance.""" diff --git a/tests/math/test_interval.py b/tests/math/test_interval.py index 97af018..e5a4a15 100644 --- a/tests/math/test_interval.py +++ b/tests/math/test_interval.py @@ -144,7 +144,5 @@ def test_contains_no_end( ) def test_contains_unbound(value: LessThanComparable) -> None: """Test if a value is within the interval with no bounds.""" - interval_no_bounds: Interval[LessThanComparable | None] = Interval( - start=None, end=None - ) + interval_no_bounds: Interval[LessThanComparable] = Interval(start=None, end=None) assert value in interval_no_bounds # any value within bounds From c53cdb188b6b27b4dee8d14ed3a5467b288f6479 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 2 Jul 2026 09:50:47 +0000 Subject: [PATCH 2/3] math: Add IntervalSet class Add IntervalSet[T] to frequenz.core.math: a normalized, immutable set of Interval[T] values that supports O(log n) membership testing via the `in` operator. On construction, IntervalSet sorts the passed intervals by start and merges any that overlap or share an inclusive endpoint. Only the normalized form is stored, so equality, hashing, and iteration are well-defined regardless of input order; callers that need the original inputs can keep them separately. Unbounded intervals are handled via the existing Interval[T] convention: a None start is treated as -infinity and a None end as +infinity during sort/merge, so for example IntervalSet((Interval(None, 5), Interval(3, None))) collapses to a single Interval(None, None). The generic container-of-containers variant discussed during design is deferred: there is no immediate use case, and adding IntervalSet alone is a fully additive step that does not foreclose a future generic type in a separate module. Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 19 ++- src/frequenz/core/math.py | 188 +++++++++++++++++++++++++ tests/math/test_interval_set.py | 235 ++++++++++++++++++++++++++++++++ 3 files changed, 438 insertions(+), 4 deletions(-) create mode 100644 tests/math/test_interval_set.py diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 5261a49..37b3d1d 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -2,7 +2,7 @@ ## Summary - +This release updates the `Interval` type to no longer include `None` in its type parameter, and introduces a new `IntervalSet` type to the `frequenz.core.math` module, which allows for efficient membership testing of normalized sets of intervals. ## Upgrading @@ -18,8 +18,19 @@ ## New Features - +- Add [`IntervalSet`][frequenz.core.math.IntervalSet] to [`frequenz.core.math`][frequenz.core.math], a normalized set of [`Interval`][frequenz.core.math.Interval] values with `O(log n)` membership testing. -## Bug Fixes + Overlapping or touching intervals are merged on construction, and `None` bounds (`-∞` / `+∞`) are handled during merging. - + Useful for allow/forbid-list style checks that previously required iterating a `Sequence[Interval]`: + + ```python + from frequenz.core.math import Interval, IntervalSet + + allowed = IntervalSet( + (Interval(1, 5), Interval(3, 10), Interval(15, 20)) + ) + assert tuple(allowed) == (Interval(1, 10), Interval(15, 20)) + assert 7 in allowed + assert 12 not in allowed + ``` diff --git a/src/frequenz/core/math.py b/src/frequenz/core/math.py index 2d535f3..067c584 100644 --- a/src/frequenz/core/math.py +++ b/src/frequenz/core/math.py @@ -4,6 +4,7 @@ """Math tools.""" import math +from collections.abc import Iterable, Iterator from dataclasses import dataclass from typing import Generic, Protocol, Self, TypeVar @@ -106,3 +107,190 @@ def __str__(self) -> str: start = "∞" if self.start is None else str(self.start) end = "∞" if self.end is None else str(self.end) return f"[{start}, {end}]" + + +@dataclass(frozen=True, repr=False) +class IntervalSet(Generic[LessThanComparableT]): + """A normalized set of intervals for efficient membership testing. + + An [`IntervalSet`][.] represents the union of a collection of + [`Interval`][..Interval] values. On construction, the passed intervals are sorted + by their start and any overlapping or touching intervals are merged into one, so + the stored [`.intervals`][.] are canonical: sorted by start and pairwise + non-overlapping. + + Membership testing via the `in` operator is `O(log n)` on the number of stored + intervals (after normalization), using binary search on the interval starts. + + Only the normalized form is stored; the original input order and any redundant + intervals are discarded. Two [`IntervalSet`][.] instances constructed from + different input sequences that represent the same union of values are equal and + hash the same. + + Note: Unbounded intervals + [`Interval`][..Interval] supports `None` as the start (meaning -∞) or the end + (meaning +∞). [`IntervalSet`][.] handles these correctly during merging: for + example, the set `{Interval(None, 5), Interval(3, None)}` normalizes to + `{Interval(None, None)}` (the whole comparable space). `None` is a bound + marker only; it is never a value in the set. + + Example: + ```python + from frequenz.core.math import Interval, IntervalSet + + allowed = IntervalSet( + (Interval(1, 5), Interval(3, 10), Interval(15, 20)) + ) + assert tuple(allowed) == (Interval(1, 10), Interval(15, 20)) + assert 7 in allowed + assert 12 not in allowed + ``` + """ + + intervals: tuple[Interval[LessThanComparableT], ...] = () + """The normalized intervals in this set: sorted by start and non-overlapping.""" + + def __post_init__(self) -> None: + """Normalize the passed intervals by sorting and merging overlapping ones.""" + object.__setattr__(self, "intervals", _sort_and_merge(self.intervals)) + + def __contains__(self, item: LessThanComparableT) -> bool: + """Check whether the value is within any interval of this set. + + Args: + item: The value to check. + + Returns: + Whether `item` is within any interval of this set. + """ + if not self.intervals: + return False + + # Binary search for the rightmost interval whose start is `<= item`. A `None` + # start means -∞, which is always `<= item`. + lo, hi = 0, len(self.intervals) + while lo < hi: + mid = (lo + hi) // 2 + start = self.intervals[mid].start + if start is None or not item < start: + lo = mid + 1 + else: + hi = mid + + idx = lo - 1 + return idx >= 0 and item in self.intervals[idx] + + def __iter__(self) -> Iterator[Interval[LessThanComparableT]]: + """Iterate over the normalized intervals in ascending order of start.""" + return iter(self.intervals) + + def __len__(self) -> int: + """Return the number of intervals in the normalized form.""" + return len(self.intervals) + + def __repr__(self) -> str: + """Return a string representation of this instance.""" + return f"IntervalSet({self.intervals!r})" + + def __str__(self) -> str: + """Return a string representation of this instance.""" + if not self.intervals: + return "∅" + return " ∪ ".join(str(iv) for iv in self.intervals) + + +def _sort_and_merge( + intervals: Iterable[Interval[LessThanComparableT]], +) -> tuple[Interval[LessThanComparableT], ...]: + """Sort intervals by start and merge overlapping or touching ones. + + A `None` start is treated as -∞ and a `None` end as +∞. Intervals are inclusive + on both bounds, so `[1, 5]` and `[5, 10]` are considered touching and merged + into `[1, 10]`. + + Args: + intervals: The intervals to normalize. + + Returns: + A tuple of sorted, pairwise non-overlapping intervals covering the same + values as the input. + """ + all_ivs = list(intervals) + if not all_ivs: + return () + + # Left-unbounded intervals (start is None) can't be sorted alongside real starts, + # so pre-merge them into at most one interval spanning -∞ up to the largest end. + # Pair the real-start intervals with their (narrowed) non-None start so the sort + # key is directly typed as `T` — no casts needed. + with_none_start: list[Interval[LessThanComparableT]] = [] + with_real_start: list[tuple[LessThanComparableT, Interval[LessThanComparableT]]] = ( + [] + ) + for iv in all_ivs: + if iv.start is None: + with_none_start.append(iv) + else: + with_real_start.append((iv.start, iv)) + + with_real_start.sort(key=lambda pair: pair[0]) + ordered_real = [pair[1] for pair in with_real_start] + + initial: list[Interval[LessThanComparableT]] = [] + if with_none_start: + if any(iv.end is None for iv in with_none_start): + initial.append(Interval(None, None)) + else: + non_none_ends = [iv.end for iv in with_none_start if iv.end is not None] + initial.append(Interval(None, max(non_none_ends))) + + ordered = initial + ordered_real + + result: list[Interval[LessThanComparableT]] = [ordered[0]] + for current in ordered[1:]: + last = result[-1] + if _end_covers_start(last.end, current.start): + new_end = _max_end(last.end, current.end) + result[-1] = Interval(last.start, new_end) + else: + result.append(current) + + return tuple(result) + + +def _end_covers_start( + end_val: LessThanComparableT | None, + start_val: LessThanComparableT | None, +) -> bool: + """Return whether `end_val >= start_val`, treating `None` as ±∞. + + Args: + end_val: An interval end, where `None` means +∞. + start_val: An interval start, where `None` means -∞. + + Returns: + Whether `end_val >= start_val` under the ±∞ convention. + """ + if end_val is None: + return True + if start_val is None: + return True + return not end_val < start_val + + +def _max_end( + a: LessThanComparableT | None, + b: LessThanComparableT | None, +) -> LessThanComparableT | None: + """Return the larger of two interval ends, where `None` means +∞. + + Args: + a: An interval end. + b: Another interval end. + + Returns: + The larger of `a` and `b` under the +∞ convention. + """ + if a is None or b is None: + return None + return b if a < b else a diff --git a/tests/math/test_interval_set.py b/tests/math/test_interval_set.py new file mode 100644 index 0000000..4cd3a52 --- /dev/null +++ b/tests/math/test_interval_set.py @@ -0,0 +1,235 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for the `IntervalSet` class.""" + +from frequenz.core.math import Interval, IntervalSet + + +def test_empty_default() -> None: + """An `IntervalSet` built with no arguments is empty and contains nothing.""" + interval_set: IntervalSet[int] = IntervalSet() + assert len(interval_set) == 0 + assert not list(interval_set) + assert 0 not in interval_set + + +def test_empty_explicit_tuple() -> None: + """An `IntervalSet` built from an empty tuple is empty.""" + interval_set: IntervalSet[int] = IntervalSet(()) + assert len(interval_set) == 0 + assert not list(interval_set) + + +def test_single_interval() -> None: + """A single interval is stored as-is and its inclusive bounds are respected.""" + interval_set = IntervalSet((Interval(1, 5),)) + assert list(interval_set) == [Interval(1, 5)] + assert 3 in interval_set + assert 1 in interval_set + assert 5 in interval_set + assert 0 not in interval_set + assert 6 not in interval_set + + +def test_overlap_merged() -> None: + """Overlapping intervals collapse into their union.""" + interval_set = IntervalSet((Interval(1, 5), Interval(3, 10))) + assert list(interval_set) == [Interval(1, 10)] + assert 7 in interval_set + assert 12 not in interval_set + + +def test_containment_merged() -> None: + """A fully-contained interval is absorbed by the larger one.""" + interval_set = IntervalSet((Interval(1, 5), Interval(2, 4))) + assert list(interval_set) == [Interval(1, 5)] + + +def test_touching_intervals_merged() -> None: + """Intervals sharing an inclusive endpoint are considered overlapping.""" + interval_set = IntervalSet((Interval(1, 5), Interval(5, 10))) + assert list(interval_set) == [Interval(1, 10)] + + +def test_disjoint_intervals_kept() -> None: + """Non-overlapping intervals remain separate.""" + interval_set = IntervalSet((Interval(1, 5), Interval(10, 20))) + assert list(interval_set) == [Interval(1, 5), Interval(10, 20)] + assert 3 in interval_set + assert 7 not in interval_set + assert 15 in interval_set + assert 25 not in interval_set + + +def test_input_order_ignored() -> None: + """Construction is independent of input order.""" + forward = IntervalSet((Interval(1, 5), Interval(10, 20))) + reversed_input = IntervalSet((Interval(10, 20), Interval(1, 5))) + assert forward == reversed_input + assert list(forward) == list(reversed_input) + + +def test_duplicates_collapsed() -> None: + """Duplicate intervals collapse into one.""" + interval_set = IntervalSet((Interval(1, 5), Interval(1, 5), Interval(1, 5))) + assert list(interval_set) == [Interval(1, 5)] + + +def test_chain_of_overlaps_merged() -> None: + """A chain of overlapping intervals collapses into a single interval.""" + interval_set = IntervalSet( + (Interval(1, 3), Interval(2, 5), Interval(4, 7), Interval(6, 9)) + ) + assert list(interval_set) == [Interval(1, 9)] + + +def test_unbounded_start() -> None: + """A left-unbounded interval contains everything up to its end.""" + interval_set: IntervalSet[int] = IntervalSet((Interval(None, 5),)) + assert -1000 in interval_set + assert 3 in interval_set + assert 5 in interval_set + assert 6 not in interval_set + + +def test_unbounded_end() -> None: + """A right-unbounded interval contains everything from its start onward.""" + interval_set: IntervalSet[int] = IntervalSet((Interval(5, None),)) + assert 5 in interval_set + assert 1000 in interval_set + assert 4 not in interval_set + + +def test_fully_unbounded_contains_every_value() -> None: + """A fully-unbounded interval contains every value in the comparable space.""" + interval_set: IntervalSet[int] = IntervalSet((Interval(None, None),)) + assert 0 in interval_set + assert -1000 in interval_set + assert 1000 in interval_set + + +def test_merge_with_unbounded_start() -> None: + """A left-unbounded interval absorbs anything it touches on the right.""" + interval_set: IntervalSet[int] = IntervalSet((Interval(None, 5), Interval(3, 10))) + assert list(interval_set) == [Interval(None, 10)] + + +def test_merge_with_unbounded_end() -> None: + """A right-unbounded interval absorbs anything it touches on the left.""" + interval_set: IntervalSet[int] = IntervalSet((Interval(5, None), Interval(3, 10))) + assert list(interval_set) == [Interval(3, None)] + + +def test_merge_covers_whole_space() -> None: + """Two overlapping half-unbounded intervals collapse to the whole space.""" + interval_set: IntervalSet[int] = IntervalSet((Interval(None, 5), Interval(3, None))) + assert list(interval_set) == [Interval(None, None)] + + +def test_multiple_unbounded_starts_pick_max_end() -> None: + """Multiple left-unbounded intervals merge into one ending at the max end.""" + interval_set: IntervalSet[int] = IntervalSet( + (Interval(None, 3), Interval(None, 5), Interval(None, 1)) + ) + assert list(interval_set) == [Interval(None, 5)] + + +def test_multiple_unbounded_ends_pick_min_start() -> None: + """Multiple right-unbounded intervals merge into one starting at the min start.""" + interval_set: IntervalSet[int] = IntervalSet( + (Interval(5, None), Interval(3, None), Interval(7, None)) + ) + assert list(interval_set) == [Interval(3, None)] + + +def test_unbounded_start_swallows_all() -> None: + """A fully-unbounded interval swallows every other interval.""" + interval_set: IntervalSet[int] = IntervalSet( + (Interval(None, None), Interval(3, 5), Interval(10, 20)) + ) + assert list(interval_set) == [Interval(None, None)] + + +def test_equality_by_normalized_form() -> None: + """Two sets with the same normalized form are equal.""" + left = IntervalSet((Interval(1, 5), Interval(10, 20))) + right = IntervalSet( + (Interval(19, 20), Interval(10, 19), Interval(1, 3), Interval(2, 5)) + ) + assert left == right + + +def test_hashable_when_bounds_are_hashable() -> None: + """`IntervalSet` values with hashable bounds are hashable and equal-hash.""" + left = IntervalSet((Interval(1, 5), Interval(10, 20))) + right = IntervalSet((Interval(10, 20), Interval(1, 5))) + assert hash(left) == hash(right) + assert {left, right} == {left} + + +def test_repr_shows_normalized_intervals() -> None: + """`repr()` shows the class name and the normalized intervals.""" + interval_set = IntervalSet((Interval(3, 10), Interval(1, 5))) + rendered = repr(interval_set) + assert rendered == "IntervalSet((Interval(1, 10),))" + + +def test_str_empty() -> None: + """`str()` of an empty set is `∅`.""" + interval_set: IntervalSet[int] = IntervalSet() + assert str(interval_set) == "∅" + + +def test_str_uses_union_notation() -> None: + """`str()` of a non-empty set joins interval `str()`s with ` ∪ `.""" + interval_set = IntervalSet((Interval(1, 5), Interval(10, 20))) + assert str(interval_set) == "[1, 5] ∪ [10, 20]" + + +def test_str_unbounded_uses_infinity() -> None: + """`str()` of a fully-unbounded set uses the infinity symbols from `Interval`.""" + interval_set: IntervalSet[int] = IntervalSet((Interval(None, None),)) + assert str(interval_set) == "[∞, ∞]" + + +def test_binary_search_across_many_intervals() -> None: + """Membership works correctly across many disjoint intervals.""" + interval_set = IntervalSet( + ( + Interval(0, 10), + Interval(20, 30), + Interval(40, 50), + Interval(60, 70), + Interval(80, 90), + ) + ) + assert 5 in interval_set + assert 25 in interval_set + assert 45 in interval_set + assert 65 in interval_set + assert 85 in interval_set + assert 15 not in interval_set + assert 35 not in interval_set + assert 55 not in interval_set + assert 75 not in interval_set + assert -1 not in interval_set + assert 100 not in interval_set + + +def test_iter_returns_normalized_order() -> None: + """Iteration yields intervals sorted by start.""" + interval_set = IntervalSet((Interval(30, 40), Interval(1, 5), Interval(10, 20))) + assert list(interval_set) == [ + Interval(1, 5), + Interval(10, 20), + Interval(30, 40), + ] + + +def test_float_bounds() -> None: + """Works with `float` bounds.""" + interval_set = IntervalSet((Interval(1.0, 5.5), Interval(3.2, 10.7))) + assert list(interval_set) == [Interval(1.0, 10.7)] + assert 4.5 in interval_set + assert 11.0 not in interval_set From ec58e68655ff032a4352fe95322df06204d492f6 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 3 Jul 2026 10:11:12 +0000 Subject: [PATCH 3/3] math: Make `Interval` and `IntervalSet` covariant `Interval` and `IntervalSet` are immutable (frozen) containers whose value type only ever appears in output positions: the `start` / `end` fields, the stored `intervals`, and iteration. Their type parameter can therefore be covariant, so an `Interval[T]` / `IntervalSet[T]` is accepted where an `Interval[S]` / `IntervalSet[S]` is expected when `T` is a subtype of `S` (for example, passing an `Interval[Power]` where an `Interval[Quantity]` is wanted). The previous invariant typing rejected these safe assignments for no benefit; contravariance is unsound, as it would allow a stored `S` to be read back as a `T`. A covariant type variable must not appear in any input position, so `__contains__` now takes `object` instead of the value type, following the convention used by the standard library containers (`Container`, `Sequence`, ...). The item is cast back to the value type internally, so runtime behaviour is unchanged: comparing a value that is not comparable to the bounds still raises `TypeError`. This reverts the soft breaking change from "Remove `None` from `Interval` type parameter": with `__contains__` widened to `object`, `None in some_interval` (and other incompatible membership checks) are no longer flagged by type checkers at call sites. Losing that call-site check is the accepted trade-off for covariance, and misuse still fails loudly at runtime. The release notes are updated accordingly. Add tests for both aspects. The covariance tests assign an `Interval[bool]` / `IntervalSet[bool]` to the `int` variant; these only type check under covariance and are enforced by mypy, since the test suite is type checked (they are no-ops at runtime). The runtime tests assert that a membership check with a value that can't be compared to the bounds raises `TypeError`. Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 10 ++- src/frequenz/core/math.py | 116 ++++++++++++++++++++++++-------- tests/math/test_interval.py | 29 ++++++++ tests/math/test_interval_set.py | 30 +++++++++ 4 files changed, 154 insertions(+), 31 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 37b3d1d..23469e8 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -2,7 +2,7 @@ ## Summary -This release updates the `Interval` type to no longer include `None` in its type parameter, and introduces a new `IntervalSet` type to the `frequenz.core.math` module, which allows for efficient membership testing of normalized sets of intervals. +This release updates the `Interval` type to no longer include `None` in its type parameter, and be covariant in its value type. It also introduces a new `IntervalSet` type to the `frequenz.core.math` module, which allows for efficient membership testing of normalized sets of intervals. ## Upgrading @@ -14,12 +14,18 @@ This release updates the `Interval` type to no longer include `None` in its type - `Interval[LessThanComparable | None]` → `Interval[LessThanComparable]` - `LessThanComparableOrNoneT` (importable name) → `LessThanComparableT` - Note that explicitly using `Interval[int | None]` still works, as `(int | None) | None` is equivalent to `int | None`, even when it is no longer needed nor recommended. Old code will mostly work, but there is one soft breaking change: `None in some_interval` and `None in some_interval_set` is now a type-check error at call sites, matching the design intent that `None` is a bound marker and never a member value. + Note that explicitly using `Interval[int | None]` still works, as `(int | None) | None` is equivalent to `int | None`, even when it is no longer needed nor recommended. ## New Features +- [`Interval`][frequenz.core.math.Interval] is now covariant in its value type, so an `Interval[T]` is accepted where an `Interval[S]` is expected when `T` is a subtype of `S`. + + To allow this, its `__contains__` now accepts any `object`, as is conventional for the standard library containers. As a consequence, membership checks such as `None in some_interval` or `"x" in Interval(1, 5)` are no longer rejected by type checkers at call sites; they still evaluate at runtime, raising a `TypeError` for values that aren't compatible with the interval's value type. + - Add [`IntervalSet`][frequenz.core.math.IntervalSet] to [`frequenz.core.math`][frequenz.core.math], a normalized set of [`Interval`][frequenz.core.math.Interval] values with `O(log n)` membership testing. + This type is also covariant in its value type, and its `__contains__` accepts any `object` for the same reasons as `Interval`. + Overlapping or touching intervals are merged on construction, and `None` bounds (`-∞` / `+∞`) are handled during merging. Useful for allow/forbid-list style checks that previously required iterating a `Sequence[Interval]`: diff --git a/src/frequenz/core/math.py b/src/frequenz/core/math.py index 067c584..aa69a6f 100644 --- a/src/frequenz/core/math.py +++ b/src/frequenz/core/math.py @@ -6,7 +6,7 @@ import math from collections.abc import Iterable, Iterator from dataclasses import dataclass -from typing import Generic, Protocol, Self, TypeVar +from typing import Generic, Protocol, Self, TypeVar, cast def is_close_to_zero(value: float, abs_tol: float = 1e-9) -> bool: @@ -39,6 +39,19 @@ def __lt__(self, other: Self, /) -> bool: """Type variable for a value that is [`LessThanComparable`][..LessThanComparable].""" +LessThanComparableT_co = TypeVar( + "LessThanComparableT_co", bound=LessThanComparable, covariant=True +) +"""Covariant type variable for a [`LessThanComparable`][..LessThanComparable] value. + +This is used by the immutable [`Interval`][..Interval] and +[`IntervalSet`][..IntervalSet] containers, which only ever produce values of this +type (via their attributes and iteration) and never consume them as inputs, so an +`Interval[T]` can be used where an `Interval[S]` is expected as long as `T` is a +subtype of `S`. +""" + + LessThanComparableOrNoneT = TypeVar( "LessThanComparableOrNoneT", bound=LessThanComparable | None ) @@ -46,12 +59,34 @@ def __lt__(self, other: Self, /) -> bool: Warning: Deprecated This type variable is deprecated and it will be removed in a future version. Use - [`LessThanComparableT`][..LessThanComparableT] instead. + [`LessThanComparableT`][..LessThanComparableT] or + [`LessThanComparableT_co`][..LessThanComparableT_co] instead. """ +def _value_type_name( + start: LessThanComparable | None, end: LessThanComparable | None +) -> str: + """Return the name of the value type for a pair of interval bounds. + + The value type is inferred at runtime from whichever bound is set (the generic + type parameter itself is erased). A fully unbounded interval (both bounds `None`) + has no value to infer the type from and returns ``"object"``; such an interval + performs no comparison, so this is never used to build an error in practice. + + Args: + start: The interval start bound, or `None` for an unbounded start. + end: The interval end bound, or `None` for an unbounded end. + + Returns: + The name of the value type, or ``"object"`` if both bounds are `None`. + """ + bound = start if start is not None else end + return "object" if bound is None else type(bound).__name__ + + @dataclass(frozen=True, repr=False) -class Interval(Generic[LessThanComparableT]): +class Interval(Generic[LessThanComparableT_co]): """An interval to test if a value is within its limits. The [`.start`][.start] and [`.end`][.end] are inclusive, meaning that the @@ -68,10 +103,10 @@ class Interval(Generic[LessThanComparableT]): the `__lt__` method to be able to compare values. """ - start: LessThanComparableT | None + start: LessThanComparableT_co | None """The start of the interval, or `None` to indicate no lower bound (-∞).""" - end: LessThanComparableT | None + end: LessThanComparableT_co | None """The end of the interval, or `None` to indicate no upper bound (+∞).""" def __post_init__(self) -> None: @@ -83,7 +118,7 @@ def __post_init__(self) -> None: f"The start ({self.start}) can't be bigger than end ({self.end})" ) - def __contains__(self, item: LessThanComparableT) -> bool: + def __contains__(self, item: object) -> bool: """Check if the value is within the range of the interval. Args: @@ -91,12 +126,23 @@ def __contains__(self, item: LessThanComparableT) -> bool: Returns: True if value is within the range, otherwise False. + + Raises: + TypeError: If `item`'s type is not compatible with the interval's value + type. """ - if self.start is not None and item < self.start: - return False - if self.end is not None and item > self.end: - return False - return True + value = cast(LessThanComparableT_co, item) + try: + if self.start is not None and value < self.start: + return False + if self.end is not None and value > self.end: + return False + return True + except TypeError: + raise TypeError( + f"Object of type {type(item).__name__!r} is not compatible with " + f"this interval's value type {_value_type_name(self.start, self.end)!r}" + ) from None def __repr__(self) -> str: """Return a string representation of this instance.""" @@ -110,7 +156,7 @@ def __str__(self) -> str: @dataclass(frozen=True, repr=False) -class IntervalSet(Generic[LessThanComparableT]): +class IntervalSet(Generic[LessThanComparableT_co]): """A normalized set of intervals for efficient membership testing. An [`IntervalSet`][.] represents the union of a collection of @@ -147,14 +193,14 @@ class IntervalSet(Generic[LessThanComparableT]): ``` """ - intervals: tuple[Interval[LessThanComparableT], ...] = () + intervals: tuple[Interval[LessThanComparableT_co], ...] = () """The normalized intervals in this set: sorted by start and non-overlapping.""" def __post_init__(self) -> None: """Normalize the passed intervals by sorting and merging overlapping ones.""" object.__setattr__(self, "intervals", _sort_and_merge(self.intervals)) - def __contains__(self, item: LessThanComparableT) -> bool: + def __contains__(self, item: object) -> bool: """Check whether the value is within any interval of this set. Args: @@ -162,25 +208,37 @@ def __contains__(self, item: LessThanComparableT) -> bool: Returns: Whether `item` is within any interval of this set. + + Raises: + TypeError: If `item`'s type is not compatible with the set's value type. """ if not self.intervals: return False - # Binary search for the rightmost interval whose start is `<= item`. A `None` - # start means -∞, which is always `<= item`. - lo, hi = 0, len(self.intervals) - while lo < hi: - mid = (lo + hi) // 2 - start = self.intervals[mid].start - if start is None or not item < start: - lo = mid + 1 - else: - hi = mid - - idx = lo - 1 - return idx >= 0 and item in self.intervals[idx] - - def __iter__(self) -> Iterator[Interval[LessThanComparableT]]: + value = cast(LessThanComparableT_co, item) + + try: + # Binary search for the rightmost interval whose start is `<= item`. A + # `None` start means -∞, which is always `<= item`. + lo, hi = 0, len(self.intervals) + while lo < hi: + mid = (lo + hi) // 2 + start = self.intervals[mid].start + if start is None or not value < start: + lo = mid + 1 + else: + hi = mid + + idx = lo - 1 + return idx >= 0 and value in self.intervals[idx] + except TypeError: + first = self.intervals[0] + raise TypeError( + f"Object of type {type(item).__name__!r} is not compatible with this " + f"set's value type {_value_type_name(first.start, first.end)!r}" + ) from None + + def __iter__(self) -> Iterator[Interval[LessThanComparableT_co]]: """Iterate over the normalized intervals in ascending order of start.""" return iter(self.intervals) diff --git a/tests/math/test_interval.py b/tests/math/test_interval.py index e5a4a15..7d9f0fd 100644 --- a/tests/math/test_interval.py +++ b/tests/math/test_interval.py @@ -146,3 +146,32 @@ def test_contains_unbound(value: LessThanComparable) -> None: """Test if a value is within the interval with no bounds.""" interval_no_bounds: Interval[LessThanComparable] = Interval(start=None, end=None) assert value in interval_no_bounds # any value within bounds + + +def test_covariance() -> None: + """Test that `Interval` is covariant in its value type. + + This is a type-level check: assigning an `Interval[bool]` to an `Interval[int]` + (`bool` is a subtype of `int`) only passes the type checker if `Interval` is + covariant; with an invariant type parameter `mypy` would reject it. At runtime + the assignment is a no-op, so the actual verification is done by `mypy`. + """ + narrower: Interval[bool] = Interval(False, True) + wider: Interval[int] = narrower + assert wider is narrower + + +@pytest.mark.parametrize("incomparable", ["a string", None, object()]) +def test_contains_incomparable_type_raises(incomparable: object) -> None: + """Test that checking membership of an incomparable value raises `TypeError`. + + `__contains__` accepts any `object` (so `Interval` can be covariant), but a value + whose type is not compatible with the interval's value type must fail loudly at + runtime with a clear error rather than silently returning a result or a cryptic + comparison error. + """ + interval = Interval(1, 5) + with pytest.raises( + TypeError, match=r"is not compatible with this interval's value" + ): + _ = incomparable in interval diff --git a/tests/math/test_interval_set.py b/tests/math/test_interval_set.py index 4cd3a52..ce37ad8 100644 --- a/tests/math/test_interval_set.py +++ b/tests/math/test_interval_set.py @@ -3,6 +3,8 @@ """Tests for the `IntervalSet` class.""" +import pytest + from frequenz.core.math import Interval, IntervalSet @@ -233,3 +235,31 @@ def test_float_bounds() -> None: assert list(interval_set) == [Interval(1.0, 10.7)] assert 4.5 in interval_set assert 11.0 not in interval_set + + +def test_covariance() -> None: + """Test that `IntervalSet` is covariant in its value type. + + This is a type-level check: assigning an `IntervalSet[bool]` to an + `IntervalSet[int]` (`bool` is a subtype of `int`) only passes the type checker if + `IntervalSet` is covariant; with an invariant type parameter `mypy` would reject + it. At runtime the assignment is a no-op, so the actual verification is done by + `mypy`. + """ + narrower: IntervalSet[bool] = IntervalSet((Interval(False, True),)) + wider: IntervalSet[int] = narrower + assert wider is narrower + + +@pytest.mark.parametrize("incomparable", ["a string", None, object()]) +def test_contains_incomparable_type_raises(incomparable: object) -> None: + """Test that checking membership of an incomparable value raises `TypeError`. + + `__contains__` accepts any `object` (so `IntervalSet` can be covariant), but a + value whose type is not compatible with the set's value type must fail loudly at + runtime with a clear error rather than silently returning a result or a cryptic + comparison error. + """ + interval_set = IntervalSet((Interval(1, 5),)) + with pytest.raises(TypeError, match=r"is not compatible with this set's value"): + _ = incomparable in interval_set