Add reviewer changes

Signed-off-by: Jack <[email protected]>

Author: jack-herrmann

src/frequenz/sdk/timeseries/_base_types.py

@@ -8,7 +8,7 @@
 from collections.abc import Callable, Iterator
 from dataclasses import dataclass
 from datetime import datetime, timezone
-from typing import Any, Generic, Protocol, Self, TypeVar, overload, runtime_checkable
+from typing import Any, Generic, Protocol, Self, TypeVar, cast, overload
 
 from ._quantities import Power, QuantityT
 
@@ -134,40 +134,22 @@ def map(
         )
 
 
-@runtime_checkable
 class Comparable(Protocol):
-    """
-    A protocol that requires the implementation of comparison methods.
+    """A protocol that requires the implementation of comparison methods.
 
     This protocol is used to ensure that types can be compared using
     the less than or equal to (`<=`) and greater than or equal to (`>=`)
     operators.
     """
 
     def __le__(self, other: Any, /) -> bool:
-        """
-        Return True if self is less than or equal to other, otherwise False.
-
-        Args:
-            other: The value to compare with.
-
-        Returns:
-            bool: True if self is less than or equal to other, otherwise False.
-        """
+        """Return whether this instance is less than or equal to `other`."""
 
     def __ge__(self, other: Any, /) -> bool:
-        """
-        Return True if self is greater than or equal to other, otherwise False.
-
-        Args:
-            other: The value to compare with.
-
-        Returns:
-            bool: True if self is greater than or equal to other, otherwise False.
-        """
+        """Return whether this instance is greater than or equal to `other`."""
 
 
-_T = TypeVar("_T")
+_T = TypeVar("_T", bound=Comparable | None)
 
 
 @dataclass(frozen=True)
@@ -190,18 +172,14 @@ def __contains__(self, item: _T) -> bool:
         Returns:
             bool: True if value is within the range, otherwise False.
         """
-        if item is None:
-            return False
-
-        assert isinstance(item, Comparable)
-
-        if self.lower is not None and self.upper is not None:
-            return self.lower <= item <= self.upper
-        if self.lower is not None:
-            return self.lower <= item
-        if self.upper is not None:
+        if self.lower is None and self.upper is None:
+            return True
+        if self.lower is None:
             return item <= self.upper
-        return False
+        if self.upper is None:
+            return self.lower <= item
+
+        return cast(Comparable, self.lower) <= item <= cast(Comparable, self.upper)
 
 
 @dataclass(frozen=True, kw_only=True)
@@ -238,9 +216,7 @@ def __contains__(self, item: Power) -> bool:
         Returns:
             bool: True if value is within the range, otherwise False.
         """
-        if not self.inclusion_bounds:
-            return False
-        if item not in self.inclusion_bounds:
+        if not self.inclusion_bounds or item not in self.inclusion_bounds:
             return False
         if self.exclusion_bounds and item in self.exclusion_bounds:
             return False

tests/timeseries/test_base_types.py

@@ -1,7 +1,7 @@
 # License: MIT
-# Copyright © 2023 Frequenz Energy-as-a-Service GmbH
+# Copyright © 2024 Frequenz Energy-as-a-Service GmbH
 
-"""Tests for battery pool."""
+"""Tests for timeseries base types."""
 
 
 from datetime import datetime
@@ -11,8 +11,7 @@
 
 
 def test_bounds_contains() -> None:
-    """
-    Test the `__contains__` method of the `Bounds` class.
+    """Test the `__contains__` method of the `Bounds` class.
 
     This method checks if a value is within the defined bounds.
     """
@@ -34,10 +33,6 @@ def test_bounds_contains() -> None:
     assert Power.from_watts(10) in bounds_no_upper  # at lower bound
     assert Power.from_watts(9) not in bounds_no_upper  # below lower bound
 
-    # test succeeds, mypy 'Unsupported operand types for in ("Power" and "Bounds[None]")'
-    # bounds_none = Bounds(lower=None, upper=None)
-    # assert Power.from_watts(15) not in bounds_none
-
 
 def test_system_bounds_contains() -> None:
     """