From cf9210bdca7b01472bbfbe624b0efdc2f3c1b7c2 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 1 Jul 2026 20:52:31 +0000 Subject: [PATCH 1/8] Add `bounds_from_proto2` conversion functions Introduce `bounds_from_proto2` and `bounds_from_proto_with_issues2` which return `frequenz.core.math.Interval[float | None]` instead of the local `Bounds` type. `Bounds` is being retired in favor of the generic `Interval` type from `frequenz-core`; these new symbols provide the migration path for callers. The existing `bounds_from_proto`/`_with_issues` remain unchanged and will be deprecated in a follow-up commit. Signed-off-by: Leandro Lucarella --- .../common/metrics/proto/v1alpha8/__init__.py | 9 +- .../common/metrics/proto/v1alpha8/_bounds.py | 46 ++++++++++ tests/metrics/proto/v1alpha8/test_bounds.py | 91 ++++++++++++++++++- 3 files changed, 144 insertions(+), 2 deletions(-) diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py b/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py index efc7c6f0..0b15c6db 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/__init__.py @@ -3,7 +3,12 @@ """Conversion of metrics enums from/to protobuf v1alpha8.""" -from ._bounds import bounds_from_proto, bounds_from_proto_with_issues +from ._bounds import ( + bounds_from_proto, + bounds_from_proto2, + bounds_from_proto_with_issues, + bounds_from_proto_with_issues2, +) from ._metric import metric_from_proto, metric_to_proto from ._metric_connection_category import ( metric_connection_category_from_proto, @@ -18,7 +23,9 @@ __all__ = [ "aggregated_metric_sample_from_proto", "bounds_from_proto", + "bounds_from_proto2", "bounds_from_proto_with_issues", + "bounds_from_proto_with_issues2", "metric_connection_category_from_proto", "metric_connection_category_to_proto", "metric_connection_from_proto_with_issues", diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py index 8e9fba4f..831b2595 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py @@ -4,6 +4,7 @@ """Loading of Bounds objects from protobuf messages.""" from frequenz.api.common.v1alpha8.metrics import bounds_pb2 +from frequenz.core.math import Interval from ..._bounds import Bounds @@ -26,6 +27,26 @@ def bounds_from_proto(message: bounds_pb2.Bounds) -> Bounds: # noqa: DOC502 ) +def bounds_from_proto2( # noqa: DOC502 + message: bounds_pb2.Bounds, +) -> Interval[float | None]: + """Create an [`Interval`][frequenz.core.math.Interval] object from a protobuf message. + + Args: + message: The protobuf message to convert. + + Returns: + The corresponding [`Interval`][frequenz.core.math.Interval] object. + + Raises: + ValueError: If the message is not valid. + """ + return Interval( + message.lower if message.HasField("lower") else None, + message.upper if message.HasField("upper") else None, + ) + + def bounds_from_proto_with_issues( message: bounds_pb2.Bounds, *, @@ -47,3 +68,28 @@ def bounds_from_proto_with_issues( except ValueError as exc: major_issues.append(str(exc)) return None + + +def bounds_from_proto_with_issues2( + message: bounds_pb2.Bounds, + *, + major_issues: list[str], + minor_issues: list[str], # pylint: disable=unused-argument +) -> Interval[float | None] | None: # noqa: DOC502 + """Create an [`Interval`][frequenz.core.math.Interval] object from a protobuf message. + + Collect issues. + + Args: + message: The protobuf message to convert. + major_issues: A list to append major issues to. + minor_issues: A list to append minor issues to. + + Returns: + The corresponding [`Interval`][frequenz.core.math.Interval] object. + """ + try: + return bounds_from_proto2(message) + except ValueError as exc: + major_issues.append(str(exc)) + return None diff --git a/tests/metrics/proto/v1alpha8/test_bounds.py b/tests/metrics/proto/v1alpha8/test_bounds.py index 0444ccde..37099057 100644 --- a/tests/metrics/proto/v1alpha8/test_bounds.py +++ b/tests/metrics/proto/v1alpha8/test_bounds.py @@ -1,7 +1,7 @@ # License: MIT # Copyright © 2025 Frequenz Energy-as-a-Service GmbH -"""Tests for Bounds class protobuf conversion.""" +"""Tests for Bounds/Interval protobuf conversion.""" from dataclasses import dataclass @@ -10,7 +10,9 @@ from frequenz.client.common.metrics.proto.v1alpha8 import ( bounds_from_proto, + bounds_from_proto2, bounds_from_proto_with_issues, + bounds_from_proto_with_issues2, ) @@ -82,6 +84,54 @@ def test_from_proto(case: ProtoConversionTestCase) -> None: assert bounds.upper == case.upper +@pytest.mark.parametrize( + "case", + [ + ProtoConversionTestCase( + name="full", + has_lower=True, + has_upper=True, + lower=-10.0, + upper=10.0, + ), + ProtoConversionTestCase( + name="no_upper_bound", + has_lower=True, + has_upper=False, + lower=-10.0, + upper=None, + ), + ProtoConversionTestCase( + name="no_lower_bound", + has_lower=False, + has_upper=True, + lower=None, + upper=10.0, + ), + ProtoConversionTestCase( + name="no_both_bounds", + has_lower=False, + has_upper=False, + lower=None, + upper=None, + ), + ], + ids=lambda case: case.name, +) +def test_from_proto2(case: ProtoConversionTestCase) -> None: + """Test conversion from protobuf message to Interval.""" + proto = bounds_pb2.Bounds() + if case.has_lower and case.lower is not None: + proto.lower = case.lower + if case.has_upper and case.upper is not None: + proto.upper = case.upper + + bounds = bounds_from_proto2(proto) + + assert bounds.start == case.lower + assert bounds.end == case.upper + + def test_from_proto_with_issues_valid() -> None: """Test bounds_from_proto_with_issues with valid bounds.""" proto = bounds_pb2.Bounds() @@ -102,6 +152,26 @@ def test_from_proto_with_issues_valid() -> None: assert not minor_issues +def test_from_proto2_with_issues_valid() -> None: + """Test bounds_from_proto_with_issues2 with valid bounds.""" + proto = bounds_pb2.Bounds() + proto.lower = -10.0 + proto.upper = 10.0 + + major_issues: list[str] = [] + minor_issues: list[str] = [] + + bounds = bounds_from_proto_with_issues2( + proto, major_issues=major_issues, minor_issues=minor_issues + ) + + assert bounds is not None + assert bounds.start == -10.0 + assert bounds.end == 10.0 + assert not major_issues + assert not minor_issues + + def test_from_proto_with_issues_invalid() -> None: """Test bounds_from_proto_with_issues with invalid bounds (lower > upper).""" proto = bounds_pb2.Bounds() @@ -122,3 +192,22 @@ def test_from_proto_with_issues_invalid() -> None: in major_issues[0] ) assert not minor_issues + + +def test_from_proto2_with_issues_invalid() -> None: + """Test bounds_from_proto_with_issues2 with invalid bounds (start > end).""" + proto = bounds_pb2.Bounds() + proto.lower = 10.0 + proto.upper = -10.0 + + major_issues: list[str] = [] + minor_issues: list[str] = [] + + bounds = bounds_from_proto_with_issues2( + proto, major_issues=major_issues, minor_issues=minor_issues + ) + + assert bounds is None + assert len(major_issues) == 1 + assert "The start (10.0) can't be bigger than end (-10.0)" in major_issues[0] + assert not minor_issues From 2144ca9de9d96d0d02b8b2d42564809de50fb242 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 1 Jul 2026 21:26:08 +0000 Subject: [PATCH 2/8] Make `Bounds` a deprecated subclass of `Interval` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite `Bounds` as a thin, deprecated subclass of `frequenz.core.math.Interval[float | None]`, exposing the historical `lower` / `upper` attribute names as deprecated `@property` aliases for `Interval.start` / `Interval.end`. `Bounds` now inherits Interval's validation, containment, string conversion, and dataclass semantics; two overrides are added: * `__eq__` — cross-compares against any `Interval` instance so `Bounds(lower=1, upper=2) == Interval(1, 2)` is `True`. Without this, dataclass-generated equality would reject mismatched types and break the migration UX. * `__hash__` — restored (Python nullifies auto-hash whenever `__eq__` is overridden) and matches Interval's hashing of `(start, end)`. Because `Bounds` now IS-A `Interval[float | None]`, code accepting `Interval[float | None]` accepts `Bounds` instances. This unlocks migrating field types on `MetricSample`, `ElectricalComponent`, and `ElectricalComponentConnection` from `Bounds` to `Interval` transparently. Two visible behavioural changes on `Bounds`: * Invalid-bounds `ValueError` message text moves from Bounds's old wording (`"Lower bound (X) must be less than or equal to upper bound (Y)"`) to Interval's (`"The start (X) can't be bigger than end (Y)"`). * `str(Bounds(lower=None, upper=None))` now returns `"[∞, ∞]"` instead of `"[None, None]"` (`Interval`'s `__str__`). We ignore the deprecation inside `bounds_from_proto()` to avoid a double deprecation message because the function itself will be deprecated in an upcoming commit. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/metrics/_bounds.py | 92 +++++++++++++------ .../common/metrics/proto/v1alpha8/_bounds.py | 12 ++- tests/metrics/proto/v1alpha8/test_bounds.py | 18 ++-- .../v1alpha8/test_sample_metric_sample.py | 4 +- tests/metrics/test_bounds.py | 89 ++++++++++++++---- 5 files changed, 159 insertions(+), 56 deletions(-) diff --git a/src/frequenz/client/common/metrics/_bounds.py b/src/frequenz/client/common/metrics/_bounds.py index de59bde5..e29373c7 100644 --- a/src/frequenz/client/common/metrics/_bounds.py +++ b/src/frequenz/client/common/metrics/_bounds.py @@ -4,42 +4,82 @@ """Definitions for bounds.""" -import dataclasses +from typing import cast +from frequenz.core.math import Interval +from typing_extensions import deprecated -@dataclasses.dataclass(frozen=True, kw_only=True) -class Bounds: + +@deprecated( + "`Bounds` is deprecated; use `frequenz.core.math.Interval[float | None]` " + "directly. `Bounds` is kept as a subclass of `Interval[float | None]` so " + "existing instances remain assignment-compatible with the new " + "`Interval`-typed fields." +) +class Bounds(Interval[float | None]): """A set of lower and upper bounds for any metric. The lower bound must be less than or equal to the upper bound. The units of the bounds are always the same as the related metric. - """ - lower: float | None = None - """The lower bound. + `Bounds` is a thin, deprecated subclass of + [`Interval[float | None]`][frequenz.core.math.Interval]. It inherits all + validation, containment, equality, and string-conversion behaviour from + `Interval` and exposes the historical `lower` / `upper` attribute names + as deprecated aliases for `start` / `end`. - If `None`, there is no lower bound. + Deprecated: + Use [`Interval[float | None]`][frequenz.core.math.Interval] from + the `frequenz-core` package instead. """ - upper: float | None = None - """The upper bound. + def __init__( + self, *, lower: float | None = None, upper: float | None = None + ) -> None: + """Create a `Bounds` instance using the deprecated `lower`/`upper` kwargs. - If `None`, there is no upper bound. - """ + Args: + lower: Deprecated alias for + [`Interval.start`][frequenz.core.math.Interval.start]. + upper: Deprecated alias for + [`Interval.end`][frequenz.core.math.Interval.end]. + """ + super().__init__(lower, upper) + + @property + @deprecated("`Bounds.lower` is deprecated; use `Interval.start` instead.") + def lower(self) -> float | None: + """Return the lower bound (deprecated alias for `start`). + + Deprecated: + Use [`start`][frequenz.core.math.Interval.start] instead. + + Returns: + The value stored in [`start`][frequenz.core.math.Interval.start]. + """ + return self.start + + @property + @deprecated("`Bounds.upper` is deprecated; use `Interval.end` instead.") + def upper(self) -> float | None: + """Return the upper bound (deprecated alias for `end`). + + Deprecated: + Use [`end`][frequenz.core.math.Interval.end] instead. + + Returns: + The value stored in [`end`][frequenz.core.math.Interval.end]. + """ + return self.end + + def __eq__(self, other: object) -> bool: + """Return whether this bound matches another interval-like object.""" + if isinstance(other, Interval): + other_interval = cast(Interval[float | None], other) + return self.start == other_interval.start and self.end == other_interval.end + return False - def __post_init__(self) -> None: - """Validate these bounds.""" - if self.lower is None: - return - if self.upper is None: - return - if self.lower > self.upper: - raise ValueError( - f"Lower bound ({self.lower}) must be less than or equal to upper " - f"bound ({self.upper})" - ) - - def __str__(self) -> str: - """Return a string representation of these bounds.""" - return f"[{self.lower}, {self.upper}]" + def __hash__(self) -> int: + """Return the hash for this interval-compatible bound.""" + return hash((self.start, self.end)) diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py index 831b2595..d22428bc 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py @@ -3,6 +3,8 @@ """Loading of Bounds objects from protobuf messages.""" +import warnings + from frequenz.api.common.v1alpha8.metrics import bounds_pb2 from frequenz.core.math import Interval @@ -21,10 +23,12 @@ def bounds_from_proto(message: bounds_pb2.Bounds) -> Bounds: # noqa: DOC502 Raises: ValueError: If the message is not valid. """ - return Bounds( - lower=message.lower if message.HasField("lower") else None, - upper=message.upper if message.HasField("upper") else None, - ) + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + return Bounds( + lower=message.lower if message.HasField("lower") else None, + upper=message.upper if message.HasField("upper") else None, + ) def bounds_from_proto2( # noqa: DOC502 diff --git a/tests/metrics/proto/v1alpha8/test_bounds.py b/tests/metrics/proto/v1alpha8/test_bounds.py index 37099057..83a6fe31 100644 --- a/tests/metrics/proto/v1alpha8/test_bounds.py +++ b/tests/metrics/proto/v1alpha8/test_bounds.py @@ -3,6 +3,7 @@ """Tests for Bounds/Interval protobuf conversion.""" +import warnings from dataclasses import dataclass import pytest @@ -80,8 +81,10 @@ def test_from_proto(case: ProtoConversionTestCase) -> None: bounds = bounds_from_proto(proto) - assert bounds.lower == case.lower - assert bounds.upper == case.upper + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + assert bounds.lower == case.lower + assert bounds.upper == case.upper @pytest.mark.parametrize( @@ -146,8 +149,10 @@ def test_from_proto_with_issues_valid() -> None: ) assert bounds is not None - assert bounds.lower == -10.0 - assert bounds.upper == 10.0 + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + assert bounds.lower == -10.0 + assert bounds.upper == 10.0 assert not major_issues assert not minor_issues @@ -187,10 +192,7 @@ def test_from_proto_with_issues_invalid() -> None: assert bounds is None assert len(major_issues) == 1 - assert ( - "Lower bound (10.0) must be less than or equal to upper bound (-10.0)" - in major_issues[0] - ) + assert "The start (10.0) can't be bigger than end (-10.0)" in major_issues[0] assert not minor_issues diff --git a/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py b/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py index 54b82520..64df5641 100644 --- a/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py +++ b/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py @@ -156,8 +156,8 @@ class _TestCase: ), expected_major_issues=[ ( - "bounds for AC_POWER_ACTIVE is invalid (Lower bound (10.0) must be " - "less than or equal to upper bound (-10.0)), ignoring these bounds" + "bounds for AC_POWER_ACTIVE is invalid (The start (10.0) can't be " + "bigger than end (-10.0)), ignoring these bounds" ) ], ), diff --git a/tests/metrics/test_bounds.py b/tests/metrics/test_bounds.py index f33b22de..8d558a97 100644 --- a/tests/metrics/test_bounds.py +++ b/tests/metrics/test_bounds.py @@ -4,8 +4,10 @@ """Tests for the Bounds class.""" import re +import warnings import pytest +from frequenz.core.math import Interval from frequenz.client.common.metrics import Bounds @@ -24,35 +26,48 @@ (0.0, 0.0), ], ) -def test_creation(lower: float, upper: float) -> None: +def test_creation(lower: float | None, upper: float | None) -> None: """Test creation of Bounds with valid values.""" - bounds = Bounds(lower=lower, upper=upper) - assert bounds.lower == lower - assert bounds.upper == upper + with pytest.deprecated_call(): + bounds = Bounds(lower=lower, upper=upper) + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + assert bounds.lower == lower + assert bounds.upper == upper def test_invalid_values() -> None: """Test that Bounds creation fails with invalid values.""" with pytest.raises( ValueError, - match=re.escape( - "Lower bound (10.0) must be less than or equal to upper bound (-10.0)" - ), + match=re.escape("The start (10.0) can't be bigger than end (-10.0)"), ): - Bounds(lower=10.0, upper=-10.0) + with pytest.deprecated_call(): + Bounds(lower=10.0, upper=-10.0) def test_str_representation() -> None: """Test string representation of Bounds.""" - bounds = Bounds(lower=-10.0, upper=10.0) + with pytest.deprecated_call(): + bounds = Bounds(lower=-10.0, upper=10.0) assert str(bounds) == "[-10.0, 10.0]" +def test_str_representation_for_none_endpoints() -> None: + """Test string representation of open Bounds.""" + with pytest.deprecated_call(): + bounds = Bounds(lower=None, upper=None) + assert str(bounds) == "[∞, ∞]" + + def test_equality() -> None: """Test equality comparison of Bounds objects.""" - bounds1 = Bounds(lower=-10.0, upper=10.0) - bounds2 = Bounds(lower=-10.0, upper=10.0) - bounds3 = Bounds(lower=-5.0, upper=5.0) + with pytest.deprecated_call(): + bounds1 = Bounds(lower=-10.0, upper=10.0) + with pytest.deprecated_call(): + bounds2 = Bounds(lower=-10.0, upper=10.0) + with pytest.deprecated_call(): + bounds3 = Bounds(lower=-5.0, upper=5.0) assert bounds1 == bounds2 assert bounds1 != bounds3 @@ -61,12 +76,54 @@ def test_equality() -> None: def test_hash() -> None: """Test that Bounds objects can be used in sets and as dictionary keys.""" - bounds1 = Bounds(lower=-10.0, upper=10.0) - bounds2 = Bounds(lower=-10.0, upper=10.0) - bounds3 = Bounds(lower=-5.0, upper=5.0) + with pytest.deprecated_call(): + bounds1 = Bounds(lower=-10.0, upper=10.0) + with pytest.deprecated_call(): + bounds2 = Bounds(lower=-10.0, upper=10.0) + with pytest.deprecated_call(): + bounds3 = Bounds(lower=-5.0, upper=5.0) bounds_set = {bounds1, bounds2, bounds3} - assert len(bounds_set) == 2 # bounds1 and bounds2 are equal + assert len(bounds_set) == 2 bounds_dict = {bounds1: "test1", bounds3: "test2"} assert len(bounds_dict) == 2 + + +def test_bounds_is_interval_subclass() -> None: + """`Bounds` is a subclass of `Interval[float | None]`.""" + assert issubclass(Bounds, Interval) + + +def test_bounds_instance_is_interval() -> None: + """A `Bounds` instance is an `Interval` instance (LSP).""" + with pytest.deprecated_call(): + b = Bounds(lower=1.0, upper=2.0) + assert isinstance(b, Interval) + + +def test_bounds_equals_interval_with_same_endpoints() -> None: + """`Bounds(lower=1, upper=2) == Interval(1, 2)`.""" + with pytest.deprecated_call(): + b = Bounds(lower=1.0, upper=2.0) + assert b == Interval[float | None](1.0, 2.0) + + +def test_bounds_lower_is_deprecated() -> None: + """Reading `Bounds.lower` emits `DeprecationWarning` and returns `.start`.""" + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + b = Bounds(lower=1.0, upper=2.0) + with pytest.deprecated_call(match=r"lower.*deprecated.*start"): + value = b.lower + assert value == b.start == 1.0 + + +def test_bounds_upper_is_deprecated() -> None: + """Reading `Bounds.upper` emits `DeprecationWarning` and returns `.end`.""" + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + b = Bounds(lower=1.0, upper=2.0) + with pytest.deprecated_call(match=r"upper.*deprecated.*end"): + value = b.upper + assert value == b.end == 2.0 From ba95932a81c24d759040acd5f77ae26eded08544 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 1 Jul 2026 21:32:24 +0000 Subject: [PATCH 3/8] Migrate `MetricSample.bounds` to `Sequence[Interval]` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the `MetricSample.bounds` field type from `list[Bounds]` to `Sequence[Interval[float | None]]`. `Sequence` (covariant) allows callers to keep passing `list[Bounds]` now that `Bounds` is a subclass of `Interval[float | None]`; it also accepts pure `list[Interval[float | None]]`, `tuple[Interval[float | None], ...]`, etc.. This is a soft-breaking type-level change: downstream code holding a variable typed `list[Bounds]` and passing it to `MetricSample` still type-checks under Sequence covariance, and code reading `sample.bounds` still supports iteration and indexing but no longer supports `.append()` or other list-mutation methods. Runtime behaviour is unchanged (the class is frozen — mutation was never legal). Allowing mutation of a frozen `dataclass` was a bug anyways, it should have never been allowed. The internal proto converter `_metric_bounds_from_proto` is rewritten in place to build `list[Interval[float | None]]` directly via `bounds_from_proto2`, so freshly-parsed samples no longer carry deprecated `Bounds` instances. Existing `bounds_from_proto` remains unchanged (deprecation lands in a follow-up commit). Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/metrics/_sample.py | 8 ++++---- .../common/metrics/proto/v1alpha8/_sample.py | 14 +++++++------- tests/metrics/test_sample_metric_sample.py | 15 +++++++++++++++ 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/frequenz/client/common/metrics/_sample.py b/src/frequenz/client/common/metrics/_sample.py index 1728b595..12d3af25 100644 --- a/src/frequenz/client/common/metrics/_sample.py +++ b/src/frequenz/client/common/metrics/_sample.py @@ -10,9 +10,9 @@ from typing import assert_never from frequenz.core.enum import Enum, deprecated_member, unique +from frequenz.core.math import Interval from .._exception import UnrecognizedValueError, UnspecifiedValueError -from ._bounds import Bounds from ._metric import Metric @@ -199,7 +199,7 @@ class MetricSample: value: float | AggregatedMetricValue | None """The value of the sampled metric.""" - bounds: list[Bounds] + bounds: Sequence[Interval[float | None]] """The bounds that apply to the metric sample. These bounds adapt in real-time to reflect the operating conditions at the time of @@ -219,9 +219,9 @@ class MetricSample: The diagram below illustrates the relationship between the bounds. ``` - bound[0].lower bound[1].upper + bound[0].start bound[1].end <-------|============|------------------|============|---------> - bound[0].upper bound[1].lower + bound[0].end bound[1].start ---- values here are disallowed and will be rejected ==== values here are allowed and will be accepted diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py b/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py index 6e804abe..c87099fb 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py @@ -6,9 +6,9 @@ from collections.abc import Sequence from frequenz.api.common.v1alpha8.metrics import bounds_pb2, metrics_pb2 +from frequenz.core.math import Interval from ....proto import datetime_from_proto -from ..._bounds import Bounds from ..._metric import Metric from ..._sample import ( AggregatedMetricValue, @@ -16,7 +16,7 @@ MetricConnectionCategory, MetricSample, ) -from ._bounds import bounds_from_proto +from ._bounds import bounds_from_proto2 from ._metric import metric_from_proto from ._metric_connection_category import metric_connection_category_from_proto @@ -130,8 +130,8 @@ def _metric_bounds_from_proto( *, major_issues: list[str], minor_issues: list[str], # pylint:disable=unused-argument -) -> list[Bounds]: - """Convert a sequence of bounds messages to a list of [`Bounds`][....Bounds]. +) -> list[Interval[float | None]]: + """Convert a sequence of bounds messages to a list of [`Interval`][frequenz.core.math.Interval]. Args: metric: The metric for which the bounds are defined, used for logging issues. @@ -140,12 +140,12 @@ def _metric_bounds_from_proto( minor_issues: A list to append minor issues to. Returns: - The resulting list of [`Bounds`][....Bounds]. + The resulting list of [`Interval`][frequenz.core.math.Interval]. """ - bounds: list[Bounds] = [] + bounds: list[Interval[float | None]] = [] for pb_bound in messages: try: - bound = bounds_from_proto(pb_bound) + bound = bounds_from_proto2(pb_bound) except ValueError as exc: metric_name = metric if isinstance(metric, int) else metric.name major_issues.append( diff --git a/tests/metrics/test_sample_metric_sample.py b/tests/metrics/test_sample_metric_sample.py index 1cc225ea..1900b3b3 100644 --- a/tests/metrics/test_sample_metric_sample.py +++ b/tests/metrics/test_sample_metric_sample.py @@ -3,9 +3,11 @@ """Tests for MetricSample class.""" +import warnings from datetime import datetime, timezone import pytest +from frequenz.core.math import Interval from frequenz.client.common import UnrecognizedValueError, UnspecifiedValueError from frequenz.client.common.metrics import ( @@ -141,6 +143,19 @@ def test_multiple_bounds(now: datetime) -> None: assert sample.bounds == bounds +def test_creation_with_interval(now: datetime) -> None: + """Constructing a MetricSample with Interval bounds is clean (no warnings).""" + with warnings.catch_warnings(): + warnings.simplefilter("error", DeprecationWarning) + sample = MetricSample( + sample_time=now, + metric=Metric.AC_POWER_ACTIVE, + value=5.0, + bounds=[Interval[float | None](-10.0, 10.0)], + ) + assert list(sample.bounds) == [Interval[float | None](-10.0, 10.0)] + + def test_get_metric_returns_known_member(now: datetime) -> None: """get_metric returns the metric when it is a known member.""" sample = MetricSample( From b5ff1603352b968b3a3b43d4de04e1b72fc9d4b9 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 1 Jul 2026 21:38:19 +0000 Subject: [PATCH 4/8] Migrate `ElectricalComponent.metric_config_bounds` to `Interval` Change the `metric_config_bounds` field on `ElectricalComponent` from `Mapping[Metric | int, Bounds]` to `Mapping[Metric | int, Interval[float | None]]`. `ElectricalComponent` is still unreleased, so this is a direct type replacement, no need for backwards-compatibility. The private converter `_metric_config_bounds_from_proto`, now calls `bounds_from_proto2` internally so freshly-parsed components no longer carry deprecated `Bounds` instances. Signed-off-by: Leandro Lucarella --- .../_electrical_component.py | 19 +++++++++++-------- .../proto/v1alpha8/_electrical_component.py | 19 ++++++++++--------- .../proto/v1alpha8/conftest.py | 17 ++++++++++------- .../test_electrical_component_base.py | 9 +++++---- 4 files changed, 36 insertions(+), 28 deletions(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py index 0135b7a9..748348a4 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -10,9 +10,10 @@ from typing import Any, Self import typing_extensions +from frequenz.core.math import Interval from ..._exception import UnspecifiedValueError -from ...metrics import Bounds, Metric +from ...metrics import Metric from ...types import Lifetime from .. import MicrogridId from ._category import ElectricalComponentCategory @@ -65,13 +66,15 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes ) """Internal guard allowing construction only via the `*_from_proto` converters.""" - metric_config_bounds: Mapping[Metric | int, Bounds] = dataclasses.field( - default_factory=dict, - # dict is not hashable, so we don't use this field to calculate the hash. This - # shouldn't be a problem since it is very unlikely that two components with all - # other attributes being equal would have different category specific metadata, - # so hash collisions should be still very unlikely. - hash=False, + metric_config_bounds: Mapping[Metric | int, Interval[float | None]] = ( + dataclasses.field( + default_factory=dict, + # dict is not hashable, so we don't use this field to calculate the hash. This + # shouldn't be a problem since it is very unlikely that two components with all + # other attributes being equal would have different category specific metadata, + # so hash collisions should be still very unlikely. + hash=False, + ) ) """The metric configuration bounds for this electrical component, keyed by metric. diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index 17998519..3e9955ee 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -11,11 +11,12 @@ from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( electrical_components_pb2, ) +from frequenz.core.math import Interval from google.protobuf.json_format import MessageToDict from ....._exception import UnrecognizedValueError -from .....metrics import Bounds, Metric -from .....metrics.proto.v1alpha8 import bounds_from_proto +from .....metrics import Metric +from .....metrics.proto.v1alpha8 import bounds_from_proto2 from .....proto import enum_from_proto from .....types import Lifetime from .....types.proto.v1alpha8 import lifetime_from_proto @@ -897,7 +898,7 @@ class _ElectricalComponentBaseData(NamedTuple): lifetime: Lifetime """The operational lifetime of the electrical component.""" - metric_config_bounds: dict[Metric | int, Bounds] + metric_config_bounds: dict[Metric | int, Interval[float | None]] """The metric configuration bounds extracted from the protobuf message.""" category_specific_info: dict[str, Any] @@ -1241,13 +1242,13 @@ def _metric_config_bounds_from_proto( *, major_issues: list[str], minor_issues: list[str], # pylint: disable=unused-argument -) -> dict[Metric | int, Bounds]: - """Convert a `MetricConfigBounds` message to a dictionary mapping `Metric` to `Bounds`. +) -> dict[Metric | int, Interval[float | None]]: + """Convert a `MetricConfigBounds` message to `Interval[float | None]` bounds. The keys of the result map are [`Metric`][frequenz.client.common.metrics.Metric] enum members (or `int` for - unrecognized values) and the values are - [`Bounds`][frequenz.client.common.metrics.Bounds] objects. + unrecognized values) and the values are [`Interval`][frequenz.core.math.Interval] + objects with `float | None` endpoints. Args: message: The `MetricConfigBounds` message. @@ -1257,7 +1258,7 @@ def _metric_config_bounds_from_proto( Returns: The resulting dictionary mapping metrics to their bounds. """ - bounds: dict[Metric | int, Bounds] = {} + bounds: dict[Metric | int, Interval[float | None]] = {} for metric_bound in message: with warnings.catch_warnings(): warnings.filterwarnings("ignore", category=DeprecationWarning) @@ -1278,7 +1279,7 @@ def _metric_config_bounds_from_proto( continue try: - bound = bounds_from_proto(metric_bound.config_bounds) + bound = bounds_from_proto2(metric_bound.config_bounds) except ValueError as exc: major_issues.append( f"metric_config_bounds for {metric} is invalid ({exc}), considering " diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py index 9a3af9a6..a763335a 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py @@ -11,9 +11,10 @@ from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( electrical_components_pb2, ) +from frequenz.core.math import Interval from google.protobuf.timestamp_pb2 import Timestamp -from frequenz.client.common.metrics import Bounds, Metric +from frequenz.client.common.metrics import Metric from frequenz.client.common.microgrid import MicrogridId from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponent, @@ -60,7 +61,9 @@ def default_component_base_data( model=DEFAULT_MODEL, category=ElectricalComponentCategory.UNSPECIFIED, lifetime=DEFAULT_LIFETIME, - metric_config_bounds={Metric.AC_ENERGY_ACTIVE: Bounds(lower=0, upper=100)}, + metric_config_bounds={ + Metric.AC_ENERGY_ACTIVE: Interval[float | None](0.0, 100.0) + }, category_specific_info={}, provides_telemetry=True, accepts_control=True, @@ -143,12 +146,12 @@ def base_data_as_proto( ) proto.operational_lifetime.CopyFrom(lifetime_pb2.Lifetime(**lifetime_dict)) if base_data.metric_config_bounds: - for metric, bounds in base_data.metric_config_bounds.items(): + for metric, interval in base_data.metric_config_bounds.items(): bounds_dict: dict[str, float] = {} - if bounds.lower is not None: - bounds_dict["lower"] = bounds.lower - if bounds.upper is not None: - bounds_dict["upper"] = bounds.upper + if interval.start is not None: + bounds_dict["lower"] = interval.start + if interval.end is not None: + bounds_dict["upper"] = interval.end metric_value = metric.value if isinstance(metric, Metric) else metric proto.metric_config_bounds.append( electrical_components_pb2.MetricConfigBounds( diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py index 5e84b532..053a5a25 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py @@ -8,9 +8,10 @@ from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( electrical_components_pb2, ) +from frequenz.core.math import Interval from google.protobuf.timestamp_pb2 import Timestamp -from frequenz.client.common.metrics import Bounds, Metric +from frequenz.client.common.metrics import Metric from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponentCategory, ) @@ -199,9 +200,9 @@ def test_metric_config_bounds_stores_unspecified_as_int() -> None: message, major_issues=major_issues, minor_issues=minor_issues ) - assert parsed[int(Metric.UNSPECIFIED.value)] == Bounds(lower=0.0, upper=1.0) + assert parsed[int(Metric.UNSPECIFIED.value)] == Interval[float | None](0.0, 1.0) assert Metric.UNSPECIFIED not in parsed - assert parsed[_UNKNOWN_METRIC_INT] == Bounds(lower=2.0, upper=3.0) - assert parsed[Metric.DC_VOLTAGE] == Bounds(lower=4.0, upper=5.0) + assert parsed[_UNKNOWN_METRIC_INT] == Interval[float | None](2.0, 3.0) + assert parsed[Metric.DC_VOLTAGE] == Interval[float | None](4.0, 5.0) assert not major_issues assert any(str(_UNKNOWN_METRIC_INT) in issue for issue in minor_issues) From 0296c20c6785d5411f249768cb50b71112986b7d Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 1 Jul 2026 21:48:55 +0000 Subject: [PATCH 5/8] Migrate `operational_lifetime` to `Interval` Change the `operational_lifetime` field on `ElectricalComponent` and `ElectricalComponentConnection` from `Lifetime` to `Interval[datetime | None]`. Both classes are still unreleased, so this is a direct type replacement, no backwards-compatibility needed. The default factory yields an unbounded `Interval[datetime | None](None, None)` matching the historical "always operational" semantics of `Lifetime()`. `lifetime_from_proto` is rewritten to return `Interval[datetime | None]` directly. The invalid-lifetime error message text in the affected tests moves from `"Start (X) must be before or equal to end (Y)"` (from `Lifetime.__post_init__`) to `"The start (X) can't be bigger than end (Y)"` (from `Interval.__post_init__`). The `Lifetime` class itself will be removed in a follow-up commit. Signed-off-by: Leandro Lucarella --- .../_electrical_component.py | 7 +++--- .../_electrical_component_connection.py | 9 ++++--- .../proto/v1alpha8/_electrical_component.py | 10 ++++---- .../_electrical_component_connection.py | 9 +++---- .../common/types/proto/v1alpha8/_lifetime.py | 12 ++++++---- .../proto/v1alpha8/conftest.py | 19 +++++++-------- .../test_electrical_component_base.py | 14 ++++++----- .../test_electrical_component_base.py | 18 +++++++------- .../test_electrical_component_connection.py | 24 ++++++++++--------- tests/types/proto/v1alpha8/test_lifetime.py | 17 +++++++++---- 10 files changed, 77 insertions(+), 62 deletions(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py index 748348a4..01f671a2 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -14,7 +14,6 @@ from ..._exception import UnspecifiedValueError from ...metrics import Metric -from ...types import Lifetime from .. import MicrogridId from ._category import ElectricalComponentCategory from ._ids import ElectricalComponentId @@ -52,7 +51,9 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes This includes both the manufacturer and the model name. """ - operational_lifetime: Lifetime = dataclasses.field(default_factory=Lifetime) + operational_lifetime: Interval[datetime | None] = dataclasses.field( + default_factory=lambda: Interval[datetime | None](None, None) + ) """The operational lifetime of this electrical component.""" _provides_telemetry: bool | None @@ -180,7 +181,7 @@ def is_operational_at(self, timestamp: datetime) -> bool: Returns: Whether this electrical component is operational at the given timestamp. """ - return self.operational_lifetime.is_operational_at(timestamp) + return timestamp in self.operational_lifetime def is_operational_now(self) -> bool: """Check whether this electrical component is currently operational. diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py index 1f0320dd..f8b452cd 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py @@ -6,7 +6,8 @@ import dataclasses from datetime import datetime, timezone -from ...types import Lifetime +from frequenz.core.math import Interval + from ._ids import ElectricalComponentId @@ -49,7 +50,9 @@ class ElectricalComponentConnection: This is the electrical component towards which the current flows. """ - operational_lifetime: Lifetime = dataclasses.field(default_factory=Lifetime) + operational_lifetime: Interval[datetime | None] = dataclasses.field( + default_factory=lambda: Interval[datetime | None](None, None) + ) """The operational lifetime of the connection.""" def __post_init__(self) -> None: @@ -59,7 +62,7 @@ def __post_init__(self) -> None: def is_operational_at(self, timestamp: datetime) -> bool: """Check whether this connection is operational at a specific timestamp.""" - return self.operational_lifetime.is_operational_at(timestamp) + return timestamp in self.operational_lifetime def is_operational_now(self) -> bool: """Whether this connection is currently operational.""" diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index 3e9955ee..60a8ce8a 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -6,6 +6,7 @@ import logging import warnings from collections.abc import Mapping, Sequence +from datetime import datetime from typing import Any, Final, NamedTuple, TypeAlias, assert_never, overload from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( @@ -18,7 +19,6 @@ from .....metrics import Metric from .....metrics.proto.v1alpha8 import bounds_from_proto2 from .....proto import enum_from_proto -from .....types import Lifetime from .....types.proto.v1alpha8 import lifetime_from_proto from ...._ids import MicrogridId from ..._battery import ( @@ -895,7 +895,7 @@ class _ElectricalComponentBaseData(NamedTuple): category: ElectricalComponentCategory | int """The category of the electrical component.""" - lifetime: Lifetime + lifetime: Interval[datetime | None] """The operational lifetime of the electrical component.""" metric_config_bounds: dict[Metric | int, Interval[float | None]] @@ -1301,7 +1301,7 @@ def _get_operational_lifetime_from_proto( *, major_issues: list[str], minor_issues: list[str], -) -> Lifetime: +) -> Interval[datetime | None]: """Get the operational lifetime from a protobuf message. Args: @@ -1310,7 +1310,7 @@ def _get_operational_lifetime_from_proto( minor_issues: A list to collect minor issues found during parsing. Returns: - The extracted operational lifetime, or an empty lifetime if the protobuf + The extracted operational lifetime, or an empty interval if the protobuf field is missing or invalid. """ if message.HasField("operational_lifetime"): @@ -1325,4 +1325,4 @@ def _get_operational_lifetime_from_proto( minor_issues.append( "missing operational lifetime, considering it always operational", ) - return Lifetime() + return Interval[datetime | None](None, None) diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py index fd35d781..3c10358a 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py @@ -4,12 +4,13 @@ """Loading of ElectricalComponentConnection objects from protobuf messages.""" import logging +from datetime import datetime from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( electrical_components_pb2, ) +from frequenz.core.math import Interval -from .....types import Lifetime from .....types.proto.v1alpha8 import lifetime_from_proto from ... import ElectricalComponentConnection, ElectricalComponentId @@ -100,7 +101,7 @@ def _get_operational_lifetime_from_proto( *, major_issues: list[str], minor_issues: list[str], -) -> Lifetime: +) -> Interval[datetime | None]: """Get the operational lifetime from a protobuf message. Args: @@ -109,7 +110,7 @@ def _get_operational_lifetime_from_proto( minor_issues: A list to collect minor issues found during parsing. Returns: - The extracted operational lifetime, or an empty lifetime if the protobuf + The extracted operational lifetime, or an empty interval if the protobuf field is missing or invalid. """ if message.HasField("operational_lifetime"): @@ -124,4 +125,4 @@ def _get_operational_lifetime_from_proto( minor_issues.append( "missing operational lifetime, considering it always operational", ) - return Lifetime() + return Interval[datetime | None](None, None) diff --git a/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py b/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py index 04f3b272..7e15d55c 100644 --- a/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py +++ b/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py @@ -3,22 +3,24 @@ """Conversion of Lifetime objects from protobuf v1alpha8 messages.""" +from datetime import datetime + from frequenz.api.common.v1alpha8.microgrid import lifetime_pb2 +from frequenz.core.math import Interval from ....proto import datetime_from_proto -from ..._lifetime import Lifetime def lifetime_from_proto( message: lifetime_pb2.Lifetime, -) -> Lifetime: - """Create a [`Lifetime`][....Lifetime] from a protobuf message. +) -> Interval[datetime | None]: + """Create an [`Interval[datetime | None]`][frequenz.core.math.Interval] from a protobuf message. Args: message: The protobuf message to convert. Returns: - The corresponding [`Lifetime`][....Lifetime] object. + The corresponding [`Interval[datetime | None]`][frequenz.core.math.Interval] object. """ start = ( datetime_from_proto(message.start_timestamp) @@ -30,4 +32,4 @@ def lifetime_from_proto( if message.HasField("end_timestamp") else None ) - return Lifetime(start_time=start, end_time=end) + return Interval[datetime | None](start, end) diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py index a763335a..8b81682f 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py @@ -25,11 +25,10 @@ _ElectricalComponentBaseData, ) from frequenz.client.common.proto import datetime_to_proto -from frequenz.client.common.types import Lifetime -DEFAULT_LIFETIME = Lifetime( - start_time=datetime(2020, 1, 1, tzinfo=timezone.utc), - end_time=datetime(2030, 1, 1, tzinfo=timezone.utc), +DEFAULT_LIFETIME = Interval[datetime | None]( + datetime(2020, 1, 1, tzinfo=timezone.utc), + datetime(2030, 1, 1, tzinfo=timezone.utc), ) DEFAULT_COMPONENT_ID = ElectricalComponentId(42) DEFAULT_MICROGRID_ID = MicrogridId(1) @@ -134,16 +133,14 @@ def base_data_as_proto( (base_data.provides_telemetry, base_data.accepts_control) ], ) - if base_data.lifetime: + if base_data.lifetime.start is not None or base_data.lifetime.end is not None: lifetime_dict: dict[str, Timestamp] = {} - if base_data.lifetime.start_time is not None: + if base_data.lifetime.start is not None: lifetime_dict["start_timestamp"] = datetime_to_proto( - base_data.lifetime.start_time - ) - if base_data.lifetime.end_time is not None: - lifetime_dict["end_timestamp"] = datetime_to_proto( - base_data.lifetime.end_time + base_data.lifetime.start ) + if base_data.lifetime.end is not None: + lifetime_dict["end_timestamp"] = datetime_to_proto(base_data.lifetime.end) proto.operational_lifetime.CopyFrom(lifetime_pb2.Lifetime(**lifetime_dict)) if base_data.metric_config_bounds: for metric, interval in base_data.metric_config_bounds.items(): diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py index 053a5a25..7cbdcc10 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py @@ -3,6 +3,8 @@ """Tests for protobuf conversion of the base/common part of electrical components.""" +from datetime import datetime + import pytest from frequenz.api.common.v1alpha8.metrics import bounds_pb2, metrics_pb2 from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( @@ -21,7 +23,6 @@ _metric_config_bounds_from_proto, _operational_mode_to_bools, ) -from frequenz.client.common.types import Lifetime from .conftest import base_data_as_proto @@ -93,7 +94,7 @@ def test_missing_category_specific_info( base_data = default_component_base_data._replace( name=None, category=ElectricalComponentCategory.UNSPECIFIED, - lifetime=Lifetime(), + lifetime=Interval[datetime | None](None, None), metric_config_bounds={}, category_specific_info={}, ) @@ -149,7 +150,8 @@ def test_invalid_lifetime( major_issues: list[str] = [] minor_issues: list[str] = [] base_data = default_component_base_data._replace( - category=ElectricalComponentCategory.CHP, lifetime=Lifetime() + category=ElectricalComponentCategory.CHP, + lifetime=Interval[datetime | None](None, None), ) proto = base_data_as_proto(base_data) proto.operational_lifetime.start_timestamp.CopyFrom( @@ -164,9 +166,9 @@ def test_invalid_lifetime( ) assert major_issues == [ - "invalid operational lifetime (Start (2023-10-02 00:00:00+00:00) must be " - "before or equal to end (2023-10-01 00:00:00+00:00)), considering it as " - "missing (i.e. always operational)" + "invalid operational lifetime (The start (2023-10-02 00:00:00+00:00) can't be " + "bigger than end (2023-10-01 00:00:00+00:00)), considering it as missing " + "(i.e. always operational)" ] assert not minor_issues assert parsed == base_data diff --git a/tests/microgrid/electrical_components/test_electrical_component_base.py b/tests/microgrid/electrical_components/test_electrical_component_base.py index 0c6ba192..9e3b105d 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/test_electrical_component_base.py @@ -4,9 +4,10 @@ """Tests for the ElectricalComponent base class and its functionality.""" from datetime import datetime, timezone -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch import pytest +from frequenz.core.math import Interval from frequenz.client.common import UnspecifiedValueError from frequenz.client.common.metrics import Bounds, Metric @@ -15,7 +16,6 @@ ElectricalComponent, ElectricalComponentId, ) -from frequenz.client.common.types import Lifetime class _TestElectricalComponent(ElectricalComponent): @@ -61,7 +61,7 @@ def test_creation_with_defaults() -> None: assert component.name is None assert component.model is None - assert component.operational_lifetime == Lifetime() + assert component.operational_lifetime == Interval[datetime | None](None, None) assert component.metric_config_bounds == {} assert component.category_specific_metadata == {} @@ -150,8 +150,8 @@ def test_str(name: str | None, expected_str: str) -> None: ) def test_operational_at(is_operational: bool) -> None: """Test active_at behavior with lifetime combinations.""" - mock_lifetime = Mock(spec=Lifetime) - mock_lifetime.is_operational_at.return_value = is_operational + mock_lifetime = MagicMock(spec=Interval) + mock_lifetime.__contains__.return_value = is_operational component = _TestElectricalComponent( id=ElectricalComponentId(1), @@ -166,7 +166,7 @@ def test_operational_at(is_operational: bool) -> None: test_time = datetime.now(timezone.utc) assert component.is_operational_at(test_time) == is_operational - mock_lifetime.is_operational_at.assert_called_once_with(test_time) + mock_lifetime.__contains__.assert_called_once_with(test_time) @patch( @@ -177,8 +177,8 @@ def test_is_operational_now(mock_datetime: Mock) -> None: """Test is_active_now method.""" now = datetime(2025, 1, 1, 12, 0, 0, tzinfo=timezone.utc) mock_datetime.now.side_effect = lambda tz: now.replace(tzinfo=tz) - mock_lifetime = Mock(spec=Lifetime) - mock_lifetime.is_operational_at.return_value = True + mock_lifetime = MagicMock(spec=Interval) + mock_lifetime.__contains__.return_value = True component = _TestElectricalComponent( id=ElectricalComponentId(1), microgrid_id=MicrogridId(1), @@ -191,7 +191,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: assert component.is_operational_now() is True - mock_lifetime.is_operational_at.assert_called_once_with(now) + mock_lifetime.__contains__.assert_called_once_with(now) COMPONENT = _TestElectricalComponent( diff --git a/tests/microgrid/electrical_components/test_electrical_component_connection.py b/tests/microgrid/electrical_components/test_electrical_component_connection.py index 5e25b764..809a3078 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_connection.py +++ b/tests/microgrid/electrical_components/test_electrical_component_connection.py @@ -4,21 +4,21 @@ """Tests for ElectricalComponentConnection class and related functionality.""" from datetime import datetime, timedelta, timezone -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch import pytest +from frequenz.core.math import Interval from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponentConnection, ElectricalComponentId, ) -from frequenz.client.common.types import Lifetime def test_creation() -> None: """Test basic ElectricalComponentConnection creation and validation.""" now = datetime.now(timezone.utc) - lifetime = Lifetime(start_time=now) + lifetime = Interval[datetime | None](now, None) connection = ElectricalComponentConnection( source_id=ElectricalComponentId(1), destination_id=ElectricalComponentId(2), @@ -50,7 +50,9 @@ def test_str() -> None: def test_equality_and_hash() -> None: """Test equality and hashing of the frozen ElectricalComponentConnection.""" - lifetime = Lifetime(start_time=datetime(2025, 1, 1, tzinfo=timezone.utc)) + lifetime = Interval[datetime | None]( + datetime(2025, 1, 1, tzinfo=timezone.utc), None + ) connection = ElectricalComponentConnection( source_id=ElectricalComponentId(1), destination_id=ElectricalComponentId(2), @@ -80,7 +82,7 @@ def test_is_operational_at_boundaries() -> None: connection = ElectricalComponentConnection( source_id=ElectricalComponentId(1), destination_id=ElectricalComponentId(2), - operational_lifetime=Lifetime(start_time=start, end_time=end), + operational_lifetime=Interval[datetime | None](start, end), ) before = start - timedelta(seconds=1) @@ -99,8 +101,8 @@ def test_is_operational_at_boundaries() -> None: ) def test_is_operational_at(lifetime_active: bool) -> None: """Test active_at behavior with lifetime.active values.""" - mock_lifetime = Mock(spec=Lifetime) - mock_lifetime.is_operational_at.return_value = lifetime_active + mock_lifetime = MagicMock(spec=Interval) + mock_lifetime.__contains__.return_value = lifetime_active connection = ElectricalComponentConnection( source_id=ElectricalComponentId(1), @@ -110,7 +112,7 @@ def test_is_operational_at(lifetime_active: bool) -> None: now = datetime.now(timezone.utc) assert connection.is_operational_at(now) == lifetime_active - mock_lifetime.is_operational_at.assert_called_once_with(now) + mock_lifetime.__contains__.assert_called_once_with(now) @patch( @@ -124,8 +126,8 @@ def test_is_operational_now(mock_datetime: Mock, lifetime_active: bool) -> None: """Test if the connection is operational at the current time.""" now = datetime(2025, 1, 1, 12, 0, 0, tzinfo=timezone.utc) mock_datetime.now.side_effect = lambda tz: now.replace(tzinfo=tz) - mock_lifetime = Mock(spec=Lifetime) - mock_lifetime.is_operational_at.return_value = lifetime_active + mock_lifetime = MagicMock(spec=Interval) + mock_lifetime.__contains__.return_value = lifetime_active connection = ElectricalComponentConnection( source_id=ElectricalComponentId(1), @@ -134,5 +136,5 @@ def test_is_operational_now(mock_datetime: Mock, lifetime_active: bool) -> None: ) assert connection.is_operational_now() is lifetime_active - mock_lifetime.is_operational_at.assert_called_once_with(now) + mock_lifetime.__contains__.assert_called_once_with(now) mock_datetime.now.assert_called_once_with(timezone.utc) diff --git a/tests/types/proto/v1alpha8/test_lifetime.py b/tests/types/proto/v1alpha8/test_lifetime.py index f30b8151..0e0c6499 100644 --- a/tests/types/proto/v1alpha8/test_lifetime.py +++ b/tests/types/proto/v1alpha8/test_lifetime.py @@ -9,6 +9,7 @@ import pytest from frequenz.api.common.v1alpha8.microgrid import lifetime_pb2 +from frequenz.core.math import Interval from google.protobuf import timestamp_pb2 from frequenz.client.common.types.proto.v1alpha8 import lifetime_from_proto @@ -78,14 +79,19 @@ def test_from_proto( lifetime = lifetime_from_proto(proto) if case.include_start: - assert lifetime.start_time == now + assert lifetime.start == now else: - assert lifetime.start_time is None + assert lifetime.start is None if case.include_end: - assert lifetime.end_time == future + assert lifetime.end == future else: - assert lifetime.end_time is None + assert lifetime.end is None + + assert lifetime == Interval[datetime | None]( + now if case.include_start else None, + future if case.include_end else None, + ) def test_from_proto_rejects_start_after_end(now: datetime, future: datetime) -> None: @@ -102,6 +108,7 @@ def test_from_proto_rejects_start_after_end(now: datetime, future: datetime) -> ) with pytest.raises( - ValueError, match=r"Start \(.*\) must be before or equal to end \(.*\)" + ValueError, + match=r"The start \(.*\) can't be bigger than end \(.*\)", ): lifetime_from_proto(proto) From a0e5c5d9e8288db2786406aae5fb065935be1cb9 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 1 Jul 2026 21:50:28 +0000 Subject: [PATCH 6/8] Remove `Lifetime` type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Delete the `Lifetime` dataclass and its tests. `Lifetime` was added in the current unreleased development cycle, and the previous commit migrated every consumer to `frequenz.core.math.Interval`, so there is no remaining user of the class. The `lifetime_from_proto` conversion function stays — it still parses `lifetime_pb2.Lifetime` protobuf messages, only its return type is now `Interval[datetime | None]`. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/types/__init__.py | 2 - src/frequenz/client/common/types/_lifetime.py | 58 ---- tests/types/proto/v1alpha8/test_lifetime.py | 4 +- tests/types/test_lifetime.py | 321 ------------------ 4 files changed, 2 insertions(+), 383 deletions(-) delete mode 100644 src/frequenz/client/common/types/_lifetime.py delete mode 100644 tests/types/test_lifetime.py diff --git a/src/frequenz/client/common/types/__init__.py b/src/frequenz/client/common/types/__init__.py index f07ef000..4bc50936 100644 --- a/src/frequenz/client/common/types/__init__.py +++ b/src/frequenz/client/common/types/__init__.py @@ -3,10 +3,8 @@ """Common types.""" -from ._lifetime import Lifetime from ._location import Location __all__ = [ - "Lifetime", "Location", ] diff --git a/src/frequenz/client/common/types/_lifetime.py b/src/frequenz/client/common/types/_lifetime.py deleted file mode 100644 index 2ac1a8fc..00000000 --- a/src/frequenz/client/common/types/_lifetime.py +++ /dev/null @@ -1,58 +0,0 @@ -# License: MIT -# Copyright © 2025 Frequenz Energy-as-a-Service GmbH - -"""Lifetime of an asset.""" - -from dataclasses import dataclass -from datetime import datetime, timezone - - -@dataclass(frozen=True, kw_only=True) -class Lifetime: - """An active operational period of an asset. - - Warning: - The [`end_time`][.end_time] timestamp indicates that the asset has been - permanently removed from service. - """ - - start_time: datetime | None = None - """The moment when the asset became operationally active. - - If `None`, the asset is considered to be active in any past moment previous to the - [`end_time`][..end_time]. - """ - - end_time: datetime | None = None - """The moment when the asset's operational activity ceased. - - If `None`, the asset is considered to be active with no plans to be deactivated. - """ - - def __post_init__(self) -> None: - """Validate this lifetime.""" - if ( - self.start_time is not None - and self.end_time is not None - and self.start_time > self.end_time - ): - raise ValueError( - f"Start ({self.start_time}) must be before or equal to end " - f"({self.end_time})" - ) - - def is_operational_at(self, timestamp: datetime) -> bool: - """Check whether this lifetime is active at a specific timestamp.""" - # Handle start time - it's not active if start_time is in the future - if self.start_time is not None and self.start_time > timestamp: - return False - # Handle end time - active up to and including end_time - if self.end_time is not None: - return self.end_time >= timestamp - # self.end_time is None, and either self.start_time is None or - # self.start_time <= timestamp, so it is active at this timestamp - return True - - def is_operational_now(self) -> bool: - """Whether this lifetime is currently active.""" - return self.is_operational_at(datetime.now(timezone.utc)) diff --git a/tests/types/proto/v1alpha8/test_lifetime.py b/tests/types/proto/v1alpha8/test_lifetime.py index 0e0c6499..9c94e52e 100644 --- a/tests/types/proto/v1alpha8/test_lifetime.py +++ b/tests/types/proto/v1alpha8/test_lifetime.py @@ -1,7 +1,7 @@ # License: MIT # Copyright © 2025 Frequenz Energy-as-a-Service GmbH -"""Tests for the Lifetime protobuf conversion.""" +"""Tests for the `lifetime_pb2.Lifetime` -> `Interval[datetime | None]` conversion.""" from dataclasses import dataclass from datetime import datetime, timezone @@ -62,7 +62,7 @@ def future(now: datetime) -> datetime: def test_from_proto( now: datetime, future: datetime, case: _ProtoConversionTestCase ) -> None: - """Test conversion from protobuf message to Lifetime.""" + """Test conversion from a `lifetime_pb2.Lifetime` message to `Interval[datetime | None]`.""" now_ts = timestamp_pb2.Timestamp() now_ts.FromDatetime(now) diff --git a/tests/types/test_lifetime.py b/tests/types/test_lifetime.py deleted file mode 100644 index f0768872..00000000 --- a/tests/types/test_lifetime.py +++ /dev/null @@ -1,321 +0,0 @@ -# License: MIT -# Copyright © 2025 Frequenz Energy-as-a-Service GmbH - -"""Tests for the Lifetime class.""" - -from dataclasses import dataclass -from datetime import datetime, timezone -from enum import Enum, auto - -import pytest - -from frequenz.client.common.types import Lifetime - - -class _Time(Enum): - """Types of time points used in tests.""" - - PAST = auto() - """A time point in the past.""" - - PRESENT = auto() - """The current time point.""" - - FUTURE = auto() - """A time point in the future.""" - - -@dataclass(frozen=True, kw_only=True) -class _LifetimeTestCase: - """Test case for Lifetime creation and validation.""" - - name: str - """The description of the test case.""" - - include_start: bool - """Whether to include start time.""" - - include_end: bool - """Whether to include end time.""" - - expected_start: bool - """Whether start should be set.""" - - expected_end: bool - """Whether end should be set.""" - - expected_operational: bool - """The expected operational state.""" - - -@dataclass(frozen=True, kw_only=True) -class _ActivityTestCase: - """Test case for Lifetime activity state.""" - - name: str - """The description of the test case.""" - - start_type: _Time | None - """The type of start time.""" - - end_type: _Time | None - """The type of end time.""" - - expected_operational: bool - """The expected operational state.""" - - -@dataclass(frozen=True, kw_only=True) -class _FixedLifetimeTestCase: - """Test case for fixed lifetime activity testing.""" - - name: str - """The description of the test case.""" - - test_time: _Time - """The type of time point to test.""" - - expected_operational: bool - """The expected operational state.""" - - -@pytest.fixture -def present() -> datetime: - """Fixture to provide current UTC time.""" - return datetime.now(timezone.utc) - - -@pytest.fixture -def past(present: datetime) -> datetime: - """Fixture to provide a past time.""" - return present.replace(year=present.year - 1) - - -@pytest.fixture -def future(present: datetime) -> datetime: - """Fixture to provide a future time.""" - return present.replace(year=present.year + 1) - - -@pytest.mark.parametrize( - "case", - [ - _LifetimeTestCase( - name="full", - include_start=True, - include_end=True, - expected_start=True, - expected_end=True, - expected_operational=True, - ), - _LifetimeTestCase( - name="only_start", - include_start=True, - include_end=False, - expected_start=True, - expected_end=False, - expected_operational=True, - ), - _LifetimeTestCase( - name="only_end", - include_start=False, - include_end=True, - expected_start=False, - expected_end=True, - expected_operational=True, - ), - _LifetimeTestCase( - name="no_dates", - include_start=False, - include_end=False, - expected_start=False, - expected_end=False, - expected_operational=True, - ), - ], - ids=lambda case: case.name, -) -def test_creation(present: datetime, future: datetime, case: _LifetimeTestCase) -> None: - """Test creating Lifetime instances with various parameters.""" - lifetime = Lifetime( - start_time=present if case.include_start else None, - end_time=future if case.include_end else None, - ) - assert (lifetime.start_time is not None) == case.expected_start - if case.expected_start: - assert lifetime.start_time == present - assert (lifetime.end_time is not None) == case.expected_end - if case.expected_end: - assert lifetime.end_time == future - assert lifetime.is_operational_now() == case.expected_operational - - -@pytest.mark.parametrize("start", [None, *_Time], ids=lambda x: f"start_{x}") -@pytest.mark.parametrize("end", [None, *_Time], ids=lambda x: f"end_{x}") -def test_validation( - past: datetime, - present: datetime, - future: datetime, - start: _Time | None, - end: _Time | None, -) -> None: - """Test validation of Lifetime parameters.""" - time_map = { - _Time.PAST: past, - _Time.PRESENT: present, - _Time.FUTURE: future, - None: None, - } - - start_time = time_map[start] - end_time = time_map[end] - - # Invalid combinations are when end is before start - should_fail = ( - start is not None - and end is not None - and ( - (start == _Time.PRESENT and end == _Time.PAST) - or (start == _Time.FUTURE and end == _Time.PAST) - or (start == _Time.FUTURE and end == _Time.PRESENT) - ) - ) - - if should_fail: - with pytest.raises( - ValueError, match=r"Start \(.*\) must be before or equal to end \(.*\)" - ): - Lifetime(start_time=start_time, end_time=end_time) - else: - lifetime = Lifetime(start_time=start_time, end_time=end_time) - # Verify the timestamps are set correctly - assert lifetime.start_time == start_time - assert lifetime.end_time == end_time - - -def test_equal_start_and_end_is_valid(present: datetime) -> None: - """Test that a Lifetime with the same start and end time is valid.""" - lifetime = Lifetime(start_time=present, end_time=present) - - assert lifetime.start_time == present - assert lifetime.end_time == present - assert lifetime.is_operational_at(present) - - -def test_equality_and_hashing(present: datetime, future: datetime) -> None: - """Test that Lifetime objects support equality and hashing.""" - lifetime1 = Lifetime(start_time=present, end_time=future) - lifetime2 = Lifetime(start_time=present, end_time=future) - lifetime3 = Lifetime(start_time=present, end_time=None) - - assert lifetime1 == lifetime2 - assert lifetime1 != lifetime3 - assert {lifetime1, lifetime2, lifetime3} == {lifetime1, lifetime3} - - -@pytest.mark.parametrize( - "case", - [ - _ActivityTestCase( - name="past_start-no_end", - start_type=_Time.PAST, - end_type=None, - expected_operational=True, - ), - _ActivityTestCase( - name="past_start-future_end", - start_type=_Time.PAST, - end_type=_Time.FUTURE, - expected_operational=True, - ), - _ActivityTestCase( - name="future_start-no_end", - start_type=_Time.FUTURE, - end_type=None, - expected_operational=False, - ), - _ActivityTestCase( - name="past_start-past_end", - start_type=_Time.PAST, - end_type=_Time.PAST, - expected_operational=False, - ), - _ActivityTestCase( - name="now_start-no_end", - start_type=_Time.PRESENT, - end_type=None, - expected_operational=True, - ), - _ActivityTestCase( - name="no_start-now_end", - start_type=None, - end_type=_Time.PRESENT, - expected_operational=True, - ), - _ActivityTestCase( - name="now_start-now_end", - start_type=_Time.PRESENT, - end_type=_Time.PRESENT, - expected_operational=True, - ), - _ActivityTestCase( - name="no_start-past_end", - start_type=None, - end_type=_Time.PAST, - expected_operational=False, - ), - ], - ids=lambda case: case.name, -) -def test_active_property( - past: datetime, future: datetime, present: datetime, case: _ActivityTestCase -) -> None: - """Test the active property of Lifetime.""" - start_time = { - _Time.PAST: past, - _Time.FUTURE: future, - _Time.PRESENT: present, - None: None, - }[case.start_type] - - end_time = { - _Time.PAST: past, - _Time.FUTURE: future, - _Time.PRESENT: present, - None: None, - }[case.end_type] - - lifetime = Lifetime(start_time=start_time, end_time=end_time) - assert lifetime.is_operational_at(present) == case.expected_operational - - -@pytest.mark.parametrize( - "case", - [ - _FixedLifetimeTestCase( - name="past", test_time=_Time.PAST, expected_operational=True - ), - _FixedLifetimeTestCase( - name="present", test_time=_Time.PRESENT, expected_operational=True - ), - _FixedLifetimeTestCase( - name="future", test_time=_Time.FUTURE, expected_operational=True - ), - ], - ids=lambda case: case.name, -) -def test_active_at_with_fixed_lifetime( - past: datetime, - future: datetime, - present: datetime, - case: _FixedLifetimeTestCase, -) -> None: - """Test active_at with different timestamps for a fixed lifetime period.""" - lifetime = Lifetime(start_time=past, end_time=future) - test_time = { - _Time.PAST: past, - _Time.PRESENT: present, - _Time.FUTURE: future, - }[case.test_time] - - assert lifetime.is_operational_at(test_time) == case.expected_operational From cfaaaeed8721c3c1f7d3a6d2c72a36dfa77a6160 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 1 Jul 2026 21:53:19 +0000 Subject: [PATCH 7/8] Deprecate old `bounds_from_proto*` functions Mark the two `Bounds`-returning proto converters `bounds_from_proto*` and `bounds_from_proto_with_issues` as `@deprecated`, pointing callers at `bounds_from_proto2` and `bounds_from_proto_with_issues2` respectively. Both new variants return `frequenz.core.math.Interval[float | None]` and are already used by every internal call site. `bounds_from_proto_with_issues` delegates to `bounds_from_proto`; the internal call is wrapped in `warnings.catch_warnings()` so callers of `_with_issues` see one `DeprecationWarning` (the outer one), not two. The internal `Bounds(...)` construction inside `bounds_from_proto` remains wrapped in `warnings.catch_warnings()` (added when `Bounds` became `@deprecated`), so the two suppressions compose correctly and each caller entry point emits exactly one warning identifying its own replacement symbol. Signed-off-by: Leandro Lucarella --- .../common/metrics/proto/v1alpha8/_bounds.py | 23 +++++++++++++++++- tests/metrics/proto/v1alpha8/test_bounds.py | 24 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py index d22428bc..a90a839f 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py @@ -7,13 +7,21 @@ from frequenz.api.common.v1alpha8.metrics import bounds_pb2 from frequenz.core.math import Interval +from typing_extensions import deprecated from ..._bounds import Bounds +@deprecated( + "`bounds_from_proto` is deprecated; use `bounds_from_proto2` " + "(returns `Interval[float | None]`) instead." +) def bounds_from_proto(message: bounds_pb2.Bounds) -> Bounds: # noqa: DOC502 """Create a [`Bounds`][....Bounds] object from a protobuf message. + Deprecated: + Use [`bounds_from_proto2`][..bounds_from_proto2] instead. + Args: message: The protobuf message to convert. @@ -51,6 +59,11 @@ def bounds_from_proto2( # noqa: DOC502 ) +@deprecated( + "`bounds_from_proto_with_issues` is deprecated; use " + "`bounds_from_proto_with_issues2` (returns `Interval[float | None] | None`) " + "instead." +) def bounds_from_proto_with_issues( message: bounds_pb2.Bounds, *, @@ -59,6 +72,10 @@ def bounds_from_proto_with_issues( ) -> Bounds | None: # noqa: DOC502 """Create a [`Bounds`][....Bounds] object from a protobuf message, collecting issues. + Deprecated: + Use [`bounds_from_proto_with_issues2`][..bounds_from_proto_with_issues2] + instead. + Args: message: The protobuf message to convert. major_issues: A list to append major issues to. @@ -68,7 +85,11 @@ def bounds_from_proto_with_issues( The corresponding [`Bounds`][....Bounds] object. """ try: - return bounds_from_proto(message) + # `bounds_from_proto` is itself `@deprecated`; suppress its warning so + # callers see only the outer `bounds_from_proto_with_issues` notice. + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + return bounds_from_proto(message) except ValueError as exc: major_issues.append(str(exc)) return None diff --git a/tests/metrics/proto/v1alpha8/test_bounds.py b/tests/metrics/proto/v1alpha8/test_bounds.py index 83a6fe31..35fc5b2d 100644 --- a/tests/metrics/proto/v1alpha8/test_bounds.py +++ b/tests/metrics/proto/v1alpha8/test_bounds.py @@ -213,3 +213,27 @@ def test_from_proto2_with_issues_invalid() -> None: assert len(major_issues) == 1 assert "The start (10.0) can't be bigger than end (-10.0)" in major_issues[0] assert not minor_issues + + +def test_bounds_from_proto_is_deprecated() -> None: + """`bounds_from_proto` emits a `DeprecationWarning` pointing at `bounds_from_proto2`.""" + proto = bounds_pb2.Bounds() + proto.lower = 1.0 + proto.upper = 2.0 + with pytest.deprecated_call(match=r"bounds_from_proto.*bounds_from_proto2"): + bounds_from_proto(proto) + + +def test_bounds_from_proto_with_issues_is_deprecated() -> None: + """`bounds_from_proto_with_issues` emits a `DeprecationWarning` pointing at its `_2` variant.""" + proto = bounds_pb2.Bounds() + proto.lower = 1.0 + proto.upper = 2.0 + major_issues: list[str] = [] + minor_issues: list[str] = [] + with pytest.deprecated_call( + match=r"bounds_from_proto_with_issues.*bounds_from_proto_with_issues2" + ): + bounds_from_proto_with_issues( + proto, major_issues=major_issues, minor_issues=minor_issues + ) From 995340f22e7c3584c0abf923c368e0a485167779 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 1 Jul 2026 21:55:46 +0000 Subject: [PATCH 8/8] Update release notes Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 0cbddae7..45deb339 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -53,6 +53,22 @@ Users are encouraged to switch from direct field access to the new `get_*()` methods (see New Features), which provide a safer way to handle unspecified or unrecognized values. +* `frequenz.client.common.metrics.Bounds` is now a deprecated subclass of [`Interval[float | None]`][frequenz.core.math.Interval]: + + * Constructing a `Bounds` instance and reading `Bounds.lower` / `Bounds.upper` both emit `DeprecationWarning`. Use [`Interval[float | None]`][frequenz.core.math.Interval] from `frequenz-core` directly, and `Interval.start` / `Interval.end` to read endpoints. + * Existing code passing `Bounds(lower=…, upper=…)` where an `Interval` is expected continues to work via LSP: `Bounds` inherits from `Interval[float | None]`, so a `Bounds` instance _is_ an `Interval` instance, and equality (`Bounds(lower=1, upper=2) == Interval(1, 2)`) is symmetric. + * `Bounds` now inherits `Interval`'s validation and string conversion, which changes two visible behaviours: + + * The `ValueError` raised for `start > end` now reads `"The start (X) can't be bigger than end (Y)"` instead of `"Lower bound (X) must be less than or equal to upper bound (Y)"`. + * `str(Bounds(lower=None, upper=None))` now returns `"[∞, ∞]"` instead of `"[None, None]"`. + +* `frequenz.client.common.metrics.MetricSample.bounds` field type changed from `list[Bounds]` to `Sequence[Interval[float | None]]`. This is a soft-breaking change: + + * `Sequence` is covariant, so existing code passing `list[Bounds]`, `list[Interval[float | None]]`, `tuple[Interval[float | None], ...]`, and similar containers continues to type-check. Code reading `sample.bounds` still supports iteration and indexing. + * Code that mutated the field with `list.append` (or any other list-only method) no longer type-checks. `MetricSample` was already frozen, so runtime mutation was never legal. + +* `frequenz.client.common.metrics.proto.v1alpha8.bounds_from_proto` and `bounds_from_proto_with_issues` are now deprecated. Use `bounds_from_proto2` and `bounds_from_proto_with_issues2` respectively — both return [`Interval[float | None]`][frequenz.core.math.Interval]. + ## New Features * Added 4 new electrical component classes for categories that previously collapsed into `UnrecognizedElectricalComponent`: @@ -81,15 +97,20 @@ * `frequenz.client.common.metrics.MetricConnection.get_category()` * `frequenz.client.common.metrics.MetricSample.get_metric()` -* Added a new `frequenz.client.common.types.Lifetime` type together with the `frequenz.client.common.types.proto.v1alpha8.lifetime_from_proto` conversion function. +* Added a new `frequenz.client.common.types.proto.v1alpha8.lifetime_from_proto` conversion function that parses a `lifetime_pb2.Lifetime` protobuf message into an [`Interval[datetime | None]`][frequenz.core.math.Interval] from `frequenz-core`. * Added a new `frequenz.client.common.types.Location` type together with the `frequenz.client.common.types.proto.v1alpha8.location_from_proto` conversion function. * Added a new `frequenz.client.common.microgrid.Microgrid` type, together with the `frequenz.client.common.microgrid.proto.v1alpha8.microgrid_from_proto` conversion function. -* Added a new `frequenz.client.common.microgrid.electrical_components` package, featuring a `ElectricalComponent` class hierarchy and its families (battery, inverter, EV charger, etc.), and `ElectricalComponentConnection`, including `v1alpha8` proto conversion functions. +* Added a new `frequenz.client.common.microgrid.electrical_components` package, featuring a `ElectricalComponent` class hierarchy and its families (battery, inverter, EV charger, etc.), and `ElectricalComponentConnection`, including `v1alpha8` proto conversion functions. Their `operational_lifetime` fields are typed [`Interval[datetime | None]`][frequenz.core.math.Interval] and `metric_config_bounds` on `ElectricalComponent` is typed `Mapping[Metric | int, Interval[float | None]]`. * Added a new `frequenz.client.common.microgrid.Microgrid` type with a raising `is_active()` method, together with the `frequenz.client.common.microgrid.proto.v1alpha8.microgrid_from_proto` conversion function. +* Added new proto conversion functions returning [`Interval[float | None]`][frequenz.core.math.Interval] to replace the deprecated `Bounds`-returning ones: + + * `frequenz.client.common.metrics.proto.v1alpha8.bounds_from_proto2` + * `frequenz.client.common.metrics.proto.v1alpha8.bounds_from_proto_with_issues2` + ## Bug Fixes