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 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/_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/__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..a90a839f 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py @@ -3,14 +3,25 @@ """Loading of Bounds objects from protobuf messages.""" +import warnings + 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. @@ -20,12 +31,39 @@ 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 + 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, ) +@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, *, @@ -34,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. @@ -43,7 +85,36 @@ 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 + + +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/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/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py index 0135b7a9..01f671a2 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -10,10 +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 ...types import Lifetime +from ...metrics import Metric from .. import MicrogridId from ._category import ElectricalComponentCategory from ._ids import ElectricalComponentId @@ -51,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 @@ -65,13 +67,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. @@ -177,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 17998519..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,18 +6,19 @@ 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 ( 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 from ...._ids import MicrogridId from ..._battery import ( @@ -894,10 +895,10 @@ 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, 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 " @@ -1300,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: @@ -1309,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"): @@ -1324,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/__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/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/metrics/proto/v1alpha8/test_bounds.py b/tests/metrics/proto/v1alpha8/test_bounds.py index 0444ccde..35fc5b2d 100644 --- a/tests/metrics/proto/v1alpha8/test_bounds.py +++ b/tests/metrics/proto/v1alpha8/test_bounds.py @@ -1,8 +1,9 @@ # License: MIT # Copyright © 2025 Frequenz Energy-as-a-Service GmbH -"""Tests for Bounds class protobuf conversion.""" +"""Tests for Bounds/Interval protobuf conversion.""" +import warnings from dataclasses import dataclass import pytest @@ -10,7 +11,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, ) @@ -78,8 +81,58 @@ 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( + "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: @@ -96,8 +149,30 @@ 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 + + +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 @@ -117,8 +192,48 @@ 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 + + +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 + + +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 + ) 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 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( diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py index 9a3af9a6..8b81682f 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, @@ -24,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) @@ -60,7 +60,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, @@ -131,24 +133,22 @@ 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, 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..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,14 +3,17 @@ """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 ( 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, ) @@ -20,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 @@ -92,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={}, ) @@ -148,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( @@ -163,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 @@ -199,9 +202,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) 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..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 @@ -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 @@ -61,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) @@ -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) 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