Add a new `CategorySpecificInfo` wrapper by llucax · Pull Request #257 · frequenz-floss/frequenz-client-common-python

llucax · 2026-07-21T12:41:39Z

An electrical component may carry category-specific info on the wire. Known fields are decoded into typed attributes on the concrete component, but anything left over — because the category is unrecognized, or because a newer API version added fields this client doesn't know yet, was either dropped or kept as a bare dict with no indication of which variant it came from. This type gives that leftover data a documented home together with the variant name needed to interpret it.

Some of this info was saved in the category_specific_info field for some component classes, but it was not saved consistently for all, and the kind of the variant was not preserved, making it hard to inspect unrecognized components or to debug mismatched categories.

This PR introduces a small frozen dataclass pairing the protobuf category_specific_info variant name (kind) with the leftover fields that this client version did not translate into typed attributes and stores the carried category specific info on every component by making the atttribute category_specific_info of type CategorySpecificInfo | None.

The protobuf category_specific_info oneof is now decoded once into its kind plus a dict of its fields, then each concrete component drops the fields it translated into typed attributes (e.g. a battery's type), keeping the kind and any leftover. The result:

recognized components keep the variant kind with an empty leftover
UnrecognizedElectricalComponent now preserves the carried info instead of dropping it — previously the escape hatch was empty for the very class that needs it most
MismatchedCategoryElectricalComponent keeps the full info (nothing was translated), recording exactly which variant disagreed with the category
components carrying no variant get None

It also stores the declared category name on mismatched components so that MismatchedCategoryElectricalComponent.__str__ can show the declared category name instead of just the raw int.

To do so MismatchedCategoryElectricalComponent now has a new category_name attribute alongside the raw category, resolved by the converter from the protobuf enum descriptor with the long ELECTRICAL_COMPONENT_CATEGORY_ prefix stripped, so __str__ renders the short, readable name: CID42:comp1:mismatched:category=BATTERY:kind=inverter

Only the mismatched component needs it: its declared category is a recognized value that its class name hides. Unrecognized components have no name to show (that is what makes them unrecognized), and every other component encodes its category in its class name.

Copilot

Pull request overview

This PR expands the client’s forward-compatibility surface by introducing a typed home for “leftover” protobuf fields (via CategorySpecificInfo) and by making numeric and bounds handling more robust/explicit across the library. In addition to the electrical-component changes described in the PR metadata, it also introduces a new FloatInt alias and a new bounds/bounds-set hierarchy with deprecations in the metrics proto converters and MetricSample.

Changes:

Add CategorySpecificInfo and plumb category_specific_info: CategorySpecificInfo | None through all electrical components (including mismatched/unrecognized cases) during proto conversion.
Introduce FloatInt = float | int and update several “float” fields/accessors to reflect the runtime numeric tower.
Add InvalidBounds / BoundsSet / InvalidBoundsSet + new accessors (MetricSample.get_bounds_set(), ElectricalComponent.get_metric_config_bounds()), and deprecate older bounds conversion/fields.

Reviewed changes

Copilot reviewed 51 out of 51 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/types/_location/test_location.py	Extends `Location` tests to cover `int` latitude/longitude via `FloatInt`.
tests/types/_location/test_invalid_longitude.py	Adds coverage that `InvalidLongitude` preserves `int` values.
tests/types/_location/test_invalid_latitude.py	Adds coverage that `InvalidLatitude` preserves `int` values.
tests/test_float.py	New tests for the `FloatInt` type alias behavior at runtime.
tests/microgrid/proto/v1alpha8/test_microgrid.py	Removes assertions about warning logging during microgrid conversion.
tests/microgrid/electrical_components/test_problematic.py	Adds string formatting tests for unrecognized/mismatched components and new `CategorySpecificInfo` usage.
tests/microgrid/electrical_components/test_power_transformer.py	Updates transformer voltage tests to accept `FloatInt`.
tests/microgrid/electrical_components/test_inverter.py	Adds `__str__` coverage for `UnrecognizedInverter`.
tests/microgrid/electrical_components/test_ev_charger.py	Adds `__str__` coverage for `UnrecognizedEvCharger`.
tests/microgrid/electrical_components/test_electrical_component_base.py	Updates base component tests for `category_specific_info` and adds bounds accessor tests.
tests/microgrid/electrical_components/test_category_specific_info.py	New unit tests for `CategorySpecificInfo` construction/equality/hashing.
tests/microgrid/electrical_components/test_battery.py	Adds `__str__` coverage for `UnrecognizedBattery`.
tests/microgrid/electrical_components/proto/v1alpha8/test_raw_storage.py	Ensures proto conversion preserves category-specific info kind/leftovers.
tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_simple.py	Verifies category mismatch preserves `CategorySpecificInfo` and adds `category_name`.
tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py	Updates base-data expectations and adds metric-config-bounds behavior tests.
tests/microgrid/electrical_components/proto/v1alpha8/conftest.py	Adjusts fixtures for `category_specific_info=None` and removes old metadata assertion.
tests/metrics/test_sample_metric_sample.py	Migrates sample tests from `bounds` to `bounds_set`, adds deprecation-path tests and new bounds-set error handling.
tests/metrics/test_sample_metric_connection.py	Updates `MetricConnection.name` semantics to empty-string default.
tests/metrics/test_sample_aggregated_value.py	Updates aggregated metric value tests to accept `FloatInt`.
tests/metrics/test_bounds.py	Removes old bounds tests (replaced by new `tests/metrics/_bounds/*`).
tests/metrics/proto/v1alpha8/test_sample_metric_sample.py	Updates proto sample conversion tests to use `bounds_set` and invalid-bounds preservation.
tests/metrics/proto/v1alpha8/test_sample_metric_connection.py	Updates proto connection test for empty-string name semantics.
tests/metrics/proto/v1alpha8/test_bounds.py	Adds coverage for new `bounds_from_proto2` and deprecation warnings on old converters.
tests/metrics/_bounds/test_invalid_bounds.py	New tests for `InvalidBounds`.
tests/metrics/_bounds/test_invalid_bounds_set.py	New tests for `InvalidBoundsSet`.
tests/metrics/_bounds/test_invalid_bounds_set_error.py	New tests for `InvalidBoundsSetError`.
tests/metrics/_bounds/test_invalid_bounds_error.py	New tests for `InvalidBoundsError`.
tests/metrics/_bounds/test_bounds.py	New/updated tests for `Bounds` behavior (contains/bool/str, `FloatInt` handling).
tests/metrics/_bounds/test_bounds_set.py	New tests for `BoundsSet` normalization/containment/str/hash behavior.
tests/metrics/_bounds/test_base_bounds.py	New tests ensuring `BaseBounds` is abstract/non-instantiable.
tests/metrics/_bounds/init.py	New package marker for bounds tests.
src/frequenz/client/common/types/_location.py	Updates location types/accessors/errors to use `FloatInt` and handle `int` in pattern matching.
src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py	Removes warning aggregation/logging from `microgrid_from_proto`.
src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py	Adds category-name lookup, introduces `CategorySpecificInfo` parsing/leftover handling, preserves invalid bounds, and switches to `bounds_from_proto2`.
src/frequenz/client/common/microgrid/electrical_components/_problematic.py	Adds/updates `__str__` for problematic components and adds `category_name` for mismatched cases.
src/frequenz/client/common/microgrid/electrical_components/_power_transformer.py	Updates transformer voltage fields to `FloatInt`.
src/frequenz/client/common/microgrid/electrical_components/_inverter.py	Adds `__str__` for `UnrecognizedInverter`.
src/frequenz/client/common/microgrid/electrical_components/_ev_charger.py	Adds `__str__` for `UnrecognizedEvCharger`.
src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py	Replaces `category_specific_metadata` with `category_specific_info`, preserves invalid bounds types, adds `get_metric_config_bounds()`, and changes `__str__` formatting.
src/frequenz/client/common/microgrid/electrical_components/_category_specific_info.py	New `CategorySpecificInfo` dataclass with custom hashing.
src/frequenz/client/common/microgrid/electrical_components/_battery.py	Adds `__str__` for `UnrecognizedBattery`.
src/frequenz/client/common/microgrid/electrical_components/init.py	Exports `CategorySpecificInfo` (and currently also exports `DefaultT`).
src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py	Switches proto sample conversion to produce `bounds_set` and preserves invalid bounds via type.
src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py	Adds `bounds_from_proto2`, and deprecates older bounds converters.
src/frequenz/client/common/metrics/proto/v1alpha8/init.py	Re-exports `bounds_from_proto2`.
src/frequenz/client/common/metrics/_sample.py	Introduces `bounds_set`, deprecates `bounds`, adds `get_bounds_set()`, and updates numeric types to `FloatInt`.
src/frequenz/client/common/metrics/_bounds.py	Adds `BaseBounds`, `InvalidBounds`, `BoundsSet`, `InvalidBoundsSet`, and related errors + membership/normalization logic.
src/frequenz/client/common/metrics/init.py	Exports the new bounds/bounds-set types and errors.
src/frequenz/client/common/_float.py	New `FloatInt` type alias module with rationale and examples.
src/frequenz/client/common/init.py	Re-exports `FloatInt`.
RELEASE_NOTES.md	Adds release notes for bounds deprecations, bounds-set APIs, and `FloatInt` (but currently omits the new category-specific info API).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 51 out of 51 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

src/frequenz/client/common/microgrid/electrical_components/_category_specific_info.py:42

CategorySpecificInfo.__hash__() hashes field values via repr(value). For nested mappings/sets, repr() can depend on insertion order / hash randomization, so two instances that compare equal (e.g. equal nested dicts with different insertion order) can end up with different hashes, violating the hash/equality contract.

        frozen_fields = tuple(
            sorted((key, repr(value)) for key, value in self.fields.items())
        )
        return hash((self.kind, frozen_fields))

src/frequenz/client/common/microgrid/electrical_components/init.py:23

DefaultT is a typing-only TypeVar used internally for overloads; re-exporting it from the package surface (__init__.py) needlessly expands the public API and makes it harder to change internal typing details later.

from ._crypto_miner import CryptoMiner
from ._diagnostic_code import ElectricalComponentDiagnosticCode
from ._electrical_component import DefaultT, ElectricalComponent
from ._electrical_component_connection import (

src/frequenz/client/common/microgrid/electrical_components/init.py:92

DefaultT (a private TypeVar) is included in __all__, effectively making it part of the public API. It should be kept internal to avoid locking in an implementation detail of type annotations.

    "CryptoMiner",
    "DcEvCharger",
    "DefaultT",
    "ElectricalComponent",
    "ElectricalComponentCategory",

llucax · 2026-07-21T12:56:05Z

This is a draft because it is based on #256, but it should be ready for review.

Now all issues in a `Microgrid` object loaded from a protobuf message are reported "on-demand" via code, so users can decide how to deal with errors, so there is no need to report them via log warnings anymore. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Introduce a `BaseBounds` frozen dataclass that will become the common supertype of the well-formed `Bounds` and the malformed variants added later in this series. It carries the `lower` and `upper` fields with their attribute docstrings, mirrors the `BaseLifetime` pattern (an abstract dataclass guarded by `__new__` against direct instantiation), and is exported from `frequenz.client.common.metrics`. `Bounds` itself is not touched yet — the field lift and the inheritance relationship come in a follow-up commit — so that this step reads as a pure addition on its own. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Lift the `lower` and `upper` fields off `Bounds` — they now come from `BaseBounds` as `float | int | None` — and declare the subclass relationship explicitly. The `__post_init__` invariant and the current `__str__` are unchanged; the only behavioral effect is that `isinstance(Bounds(...), BaseBounds)` now returns `True`, matching what subsequent commits rely on for the `Bounds | InvalidBounds` union. The class docstring gains a `Note:` spelling out the `ValueError` raised on invariant violation. The forward reference to `InvalidBounds` is deferred until that type actually exists to keep `mkdocs --strict` happy. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Drop the space after the comma in the compact rendering so bounds now print as e.g. `[-10.0,10.0]`, aligning with `Lifetime`'s `({start},{end}]` shape. Keeping the two `__str__` methods visually consistent matters because the upcoming `InvalidBounds` will wrap this same rendering in the `<invalid:...>` marker, and any drift between the two would leak into that composed form. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Introduce a second `BaseBounds` leaf that carries bounds data received from the wire even when it violates the `Bounds` invariant (`lower <= upper`). The type enforces no invariants of its own, so callers can inspect the raw values without accidentally treating them as a valid range — while `isinstance(x, Bounds)` still guarantees the invariant holds for well-formed data. The class ships a dedicated `__str__` that reuses the compact `[lower,upper]` shape and wraps it in the shared `<invalid:...>` marker used by other invalid types in this codebase. Now that the sibling exists, the deferred forward references from `BaseBounds` and `Bounds` are filled in — both docstrings link back to `InvalidBounds` — and the type is re-exported alongside `BaseBounds` and `Bounds` from `frequenz.client.common.metrics`. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Introduce a third `BaseBounds` leaf for the specific case of a wire entry that named a metric but carried no bounds data at all (e.g. a `MetricConfigBounds` entry without a `config_bounds` field). Making this distinct from an unbounded well-formed `Bounds(lower=None, upper=None)` lets callers tell "the server said this metric has no bounds" from "the server forgot to tell us the bounds" and react accordingly. `MissingBounds` is a subclass of `InvalidBounds` on purpose. Container types stay simple — `Bounds | InvalidBounds` still covers everything a converter can produce — and users who care about the missing case narrow with a `case MissingBounds()` arm before the generic `case InvalidBounds()` in a `match` statement. Instances always carry `lower` and `upper` as `None`; `__post_init__` rejects any attempt to attach values so the type's only affordance stays the pure "missing" signal. `__str__` returns the greppable `<invalid:missing>` marker. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Introduce `InvalidBoundsError` as the exception that upcoming `get_metric_config_bounds()`-style semantic accessors will raise when the underlying wire data was parsed into an `InvalidBounds` (or its `MissingBounds` subclass). It mirrors `InvalidLifetimeError` field-for-field: `instance` and `attr_name` come from the shared `InvalidAttributeError` base, plus a typed `.bounds: InvalidBounds` attribute so callers can inspect the offending values in a `try/except`. The default message embeds `repr(bounds)` so the offending `lower`/`upper` values (the full `InvalidBounds(...)` dataclass representation, or a `MissingBounds` shape) are visible without the caller having to unpack `.bounds`, and a `message` override is accepted so callers with more context can supply their own text. Because `InvalidAttributeError` already inherits from `ValueError`, existing catch-all `except ValueError:` sites keep working — the new type only adds structure for callers that want it. Exported from `frequenz.client.common.metrics` next to `InvalidBounds` and `MissingBounds`. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Introduce the wire converter callers should migrate to: `bounds_from_proto2` returns `Bounds | InvalidBounds` and therefore lifts the "did the server send a valid range?" question into the type system instead of relying on a `ValueError` escape hatch. The implementation mirrors `lifetime_from_proto` on `err-lifetime` — extract `lower`/`upper` with `HasField`, try to build a well-formed `Bounds`, and fall back to `InvalidBounds` when the invariant fires — so both converters read the same way. A present-but-empty message still becomes an unbounded `Bounds()`, matching what the legacy `bounds_from_proto` produces today; the new function never returns `MissingBounds` because "there was no bounds message at all" is a fact only the containing message knows about, and classifying it belongs to the caller that saw the missing field, not this converter. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Mark `bounds_from_proto` with `typing_extensions.deprecated` and point users at `bounds_from_proto2` in a `Warning: Deprecated` docstring admonition, matching the pattern already used by `delivery_area_from_proto`. The body is unchanged — external callers that ignore the warning still get exactly the same `Bounds` object (or `ValueError`) as before. Two internal call sites are converted so the library itself does not depend on the deprecated function anymore: * `_sample.py::_metric_bounds_from_proto` switches to `bounds_from_proto2` and a `match` over `Bounds` / `InvalidBounds` with `assert_never` for exhaustiveness. Observable behavior is preserved: well-formed entries land in the returned list, invalid ones are skipped and reported via `major_issues`. The reported message keeps the shape `bounds for <metric> is invalid (<detail>), ignoring these bounds`; the parenthetical now renders the `InvalidBounds.__str__` marker (`<invalid:[lower,upper]>`) instead of the raw exception text, which is a strict quality improvement (the marker is greppable and consistent with the other invalid types) — the affected assertion in `test_sample_metric_sample.py` is updated to match. * `bounds_from_proto_with_issues` inlines the `Bounds(...)` construction so it no longer routes through the deprecated helper. Its released contract (return the `Bounds`, or `None` + a `ValueError` string appended to `major_issues`) is byte-for-byte preserved. The direct call sites in `tests/metrics/proto/v1alpha8/test_bounds.py` are wrapped in `pytest.deprecated_call(match="bounds_from_proto2")` so the emitted `DeprecationWarning` is asserted rather than merely tolerated. `_electrical_component.py::_metric_config_bounds_from_proto` still calls `bounds_from_proto` internally; deprecating the call site under `src/frequenz/client/common/microgrid/` is deferred to the Phase B follow-up so this commit stays scoped to the metrics layer. Runtime callers see the outer `warnings.catch_warnings()` wrap in `_electrical_component_base_from_proto_with_issues`; the pytest `filterwarnings = ["once::DeprecationWarning", ...]` setting keeps the direct microgrid unit test that reaches into the private helper from turning the leaked warning into an error. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Mark `bounds_from_proto_with_issues` with `typing_extensions.deprecated` and point users at `bounds_from_proto2` in a `Warning: Deprecated` docstring admonition. Callers should read the returned type (`Bounds | InvalidBounds`) directly instead of the string side-channel in `major_issues`; the new converter carries the same information at the type level and does not need the caller to hand-inspect a list. The function body is unchanged so external callers that keep it around during the transition still get the exact behavior they've been relying on. The two direct call sites in `test_bounds.py` wrap the call in `pytest.deprecated_call(match="bounds_from_proto2")` so the emitted `DeprecationWarning` is asserted rather than tolerated, and a dedicated `test_from_proto_with_issues_emits_deprecation_warning` locks the warning message down as the last shipped guarantee. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

`ElectricalComponent.metric_config_bounds` is unreleased (introduced on this branch), so its type changes in place — no deprecation dance needed. Now that validity is encoded in the type system by `Bounds | InvalidBounds`, we can drop the side-channel string reports that used to describe malformed or missing entries and let the map itself carry the information. This mirrors the `err-lifetime` cleanup in `dc6a541`. Concretely: * `ElectricalComponent.metric_config_bounds` becomes `Mapping[Metric | int, Bounds | InvalidBounds]`. The field docstring spells out that malformed values are preserved as `InvalidBounds` and that entries whose `config_bounds` was not set become the `MissingBounds` subclass so callers can tell "server said unbounded" from "server forgot to send bounds". `MissingBounds` deliberately does not appear in the annotation — it is an `InvalidBounds` subclass, and keeping the union two-armed lets `match`/`isinstance` on `InvalidBounds` catch both cases while a `case MissingBounds()` arm distinguishes them when desired. * `_ElectricalComponentBaseData.metric_config_bounds` follows the same type change, with its attribute docstring updated to match. * `_metric_config_bounds_from_proto` loses the `major_issues` / `minor_issues` keyword parameters entirely. For each entry: * the metric key handling is unchanged in effect (unspecified → raw `int` `0`, unrecognized → raw `int`, the existing `warnings.catch_warnings` suppression for the deprecated `Metric.UNSPECIFIED` member is kept), but the "unrecognized metric" minor issue is gone — the plain `int` key is already the signal; * a missing `config_bounds` field stores `MissingBounds()` instead of logging a major issue and dropping the entry; * a present `config_bounds` runs through `bounds_from_proto2` and its result (`Bounds | InvalidBounds`) is stored directly — invalid entries are no longer silently dropped; * a duplicated metric on the wire keeps the last entry silently, per proto3 map semantics (the previous "using the last one" major-issue string is dropped; a comment explains the semantics). * The `_electrical_component_base_from_proto_with_issues` caller drops the `major_issues=`/`minor_issues=` kwargs on the `_metric_config_bounds_from_proto` call; the rest of the component-level `_with_issues` machinery is untouched (lifetime and category checks still surface issues as before). The direct-caller tests in `test_electrical_component_base.py` are rewritten around the new signature: `stores_unspecified_as_int` drops the issue-list plumbing; new cases lock down that invalid bounds surface as an `InvalidBounds` entry (values preserved, not equal to any `Bounds`), that a missing `config_bounds` yields `MissingBounds()`, and that duplicated metrics keep the last entry. No other `metric_config_bounds` assertion in the wider suite depended on the removed issue strings, so no other tests changed. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Now that `ElectricalComponent.metric_config_bounds` is `Mapping[Metric | int, Bounds | InvalidBounds]`, callers that want a valid `Bounds` for a given metric need to narrow the union themselves on every access — which is easy to get wrong and easy to skip. Add two higher-level accessors that do the narrowing once, mirroring the pattern established by `Microgrid.get_delivery_area()` / `get_delivery_area_or_none()` on this branch and `ElectricalComponent.get_operational_lifetime()` on `err-lifetime`: * `get_metric_config_bounds(metric: Metric) -> Bounds` — a plain `self.metric_config_bounds[metric]` lookup (so a `KeyError` still propagates naturally when no bounds are configured for the metric, documented with a `# noqa: DOC502` per the repo precedent), then a `match` that returns the `Bounds` unchanged and turns any `InvalidBounds` — including its `MissingBounds` subclass — into an `InvalidBoundsError`. The default message is overridden to include the metric name so the exception is self-describing without the caller having to unpack `.bounds`. * `get_metric_config_bounds_or_none(metric: Metric) -> Bounds | None` — same shape but backed by `.metric_config_bounds.get(metric)`, so an absent metric returns `None` instead of raising `KeyError`. Malformed or missing bounds still raise `InvalidBoundsError`; only the "no entry at all" case falls through as `None`. Both accessors take `metric: Metric` only. Raw-`int` keys represent forward-compat or unspecified values that never carry semantic bounds, so exposing them here would just create a footgun; callers who need that surface can read the mapping directly. With the getters in place, the `metric_config_bounds` field docstring gains a `Tip:` pointing at both of them (matching the shape used for `operational_lifetime` on `err-lifetime`), and `InvalidBoundsError` is imported from `...metrics`. Tests live next to the existing `provides_telemetry` / `accepts_control` accessor tests in `tests/microgrid/electrical_components/test_electrical_component_base.py` and cover every arm: valid entry returns the `Bounds`, absent metric raises `KeyError` (or returns `None` for the `_or_none` variant), an `InvalidBounds` entry raises `InvalidBoundsError` with `.bounds` preserving the values and the metric named in the message, a `MissingBounds` entry raises `InvalidBoundsError` with `.bounds` still being a `MissingBounds`. A small `_make_component` helper keeps the test bodies focused on the accessor behavior. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

`src/.../metrics/_bounds.py` now exposes five public symbols, so the test file grew too large. Split it following the repository convention: tests/metrics/test_bounds.py -> tests/metrics/_bounds/test_<thing>.py Test bodies and assertions are unchanged. No shared fixtures are needed, so no `conftest.py`. Test names drop redundant type prefixes now that the file itself names the target (e.g. `test_missing_bounds_rejects_values` → `test_rejects_values`, `test_invalid_bounds_str_representation` → `test_str_representation`). Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

A `MetricConfigBounds` entry without a `config_bounds` field is not an error: it is the explicit wire encoding of an unbounded metric. Protobuf does not serialize default values, so an all-defaults `Bounds` message is always absent from the wire — there is no other way to encode "unbounded" than omitting the message. Treating that as a defect was wrong. Drop the `MissingBounds` class and load entries without `config_bounds` as an unbounded `Bounds()` instead. This also simplifies the converter: an unset `config_bounds` field reads as the default empty message, which `bounds_from_proto2` already turns into an unbounded `Bounds()`, so the `HasField()` special case disappears entirely. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Now that an absent metric-config entry means "explicitly unbounded", forcing users to special-case absence (catching `KeyError` or checking for `None`) is just friction: they almost always want the bounds and an unbounded `Bounds()` is a perfectly usable answer. Return an unbounded `Bounds()` for absent metrics instead of raising, and accept a keyword-only `default` (like `dict.get()`) for callers that do need to detect absence — `default=None` recovers the old `get_metric_config_bounds_or_none()` behavior exactly, so that accessor is removed. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Give `Bounds` membership-testing capability, so callers can write `value in bounds` to check whether a value falls within the range. The semantics are: * both bounds are inclusive (`lower <= value <= upper`), * a `None` bound means unbounded in that direction, and * `None` is a bound marker only, never a value, so `None in bounds` is always `False`. The method lives on `Bounds` alone, not on `BaseBounds` or `InvalidBounds`: malformed bounds must not be range-checked — preserving them as `InvalidBounds` is precisely so callers inspect the raw values instead of testing membership against a broken range. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Let `Bounds` answer "do these bounds restrict anything?" so that a `Bounds | None` field can be tested uniformly: both `None` and a fully unbounded `Bounds()` become falsy, so `if not bounds:` reads as "unbounded". Any set `lower` or `upper` makes the bounds truthy. `is_bounded()` is the explicit spelling of that truthiness, following the method style used elsewhere (e.g. `Microgrid.is_active()`), for callers who prefer a named predicate over relying on `bool()`. Emptiness is decided by `is not None`, not by the value's own truthiness, so `Bounds(lower=0.0, upper=0.0)` is correctly bounded rather than being mistaken for unbounded because `0.0` is falsy. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Introduce a normalized union of `Bounds` for efficient membership testing. On construction the bounds are sorted by lower value and overlapping or touching bounds are merged (inclusive, so `[1, 5]` and `[5, 10]` become `[1, 10]`), so the stored `bounds` are canonical: sorted and pairwise non-overlapping. Membership checks perform a binary search over the normalized bounds. `BoundsSet` is a domain-specialized set, not a mathematical one: the empty set is the *unbounded* set. It contains every value and is falsy, so `not bounds_set` reliably means "unbounded". To keep that invariant honest even when unboundedness arrives split across bounds (e.g. the wire sends `[None, 5]` and `[3, None]`), a union that covers the whole space collapses to the empty set, giving unboundedness a single canonical representation. `__contains__` special-cases the empty set to contain everything, and `__bool__` / `is_bounded()` report emptiness. The type deliberately omits `__iter__` and `__len__`: the normalized sequence is public as `BoundsSet.bounds`, and querying membership by iterating it would disagree with `value in bounds_set` for the unbounded set, so `in` is kept the single authoritative operation. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Add the set-level counterpart to `InvalidBounds`, mirroring the `Bounds` / `InvalidBounds` split one level up. A collection of bounds that contains any `InvalidBounds` cannot be normalized into a well-formed `BoundsSet` — malformed ranges have no meaningful sort or merge order — so it is preserved as an `InvalidBoundsSet` instead of silently dropping the bad entries. The type keeps all of the raw bounds (valid and invalid alike) in their original wire order, with no normalization, so callers can inspect exactly what was received. It deliberately provides no membership test: range-checking malformed data is exactly the mistake this type exists to prevent. It is truthy by default, since malformed data is not the same as "unbounded" — only an empty `BoundsSet` means unbounded. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Replace `MetricSample.bounds: list[Bounds]` with `bounds_set: BoundsSet | InvalidBoundsSet`. The metric bounds are a union of ranges, so a normalized `BoundsSet` models them better than a raw list, and — mirroring `bounds_from_proto2` returning `Bounds | InvalidBounds` — malformed wire data is now preserved as an `InvalidBoundsSet` instead of being silently dropped. `bounds` is kept as deprecated for backwards-compatibility. A hand-written `__init__` still accepts the deprecated `bounds=` keyword (emitting a `DeprecationWarning` and building a `BoundsSet` from it), and a deprecated read-only `bounds` property returns the valid `Bounds` from `bounds_set`, so both existing readers and constructors keep working (with warnings). `init=False` plus a manual `__init__` is required because an `InitVar` named `bounds` would collide with the `bounds` property. We could have named the new field `bounds2`, but we decided for `bounds_set` instead: it is a permanent, self-documenting name, so the next minor just drops the deprecated `bounds` property and parameter and leaves `bounds_set` in place — no rename needed. Since the new field is not a 1-1 translation of the protocol message (it goes through normalization) it makes sense to have a different name than the protobuf field. The compatibility property returns only the valid bounds (dropping malformed ones, as the old field effectively did) but the merged, normalized bounds rather than the raw wire list; that small divergence is acceptable for a deprecated shim. On the proto side `_metric_bounds_from_proto` becomes `_bounds_set_from_proto`, returning `BoundsSet | InvalidBoundsSet`. It no longer needs the `metric` argument or the `major_issues` / `minor_issues` side channels: validity is encoded in the returned type (as done for `metric_config_bounds`), so the "bounds ... is invalid, ignoring these bounds" major issue is gone and the invalid bounds are preserved instead of dropped. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Add the set-level counterpart to `InvalidBoundsError`, for a semantic accessor that resolves a `BoundsSet | InvalidBoundsSet` field to a valid `BoundsSet` and instead sees an `InvalidBoundsSet`. It carries the offending set on its `bounds_set` attribute so callers can inspect the raw wire data, and is an `InvalidAttributeError` (hence also a `ValueError`) like the rest of the accessor errors. This is the error `MetricSample.get_bounds_set()` will raise; it is added on its own first. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

`MetricSample.bounds_set` is the lower-level, forward-compatible field: callers reading it must narrow the `BoundsSet | InvalidBoundsSet` union themselves on every access, which is easy to get wrong and easy to skip. Add a higher-level accessor that does the narrowing once: `get_bounds_set()` returns the `BoundsSet` unchanged for well-formed data and turns an `InvalidBoundsSet` into an `InvalidBoundsSetError`, whose `bounds_set` attribute preserves the raw set for inspection. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

`float("nan")` compares `False` against everything, so `nan < lower` and `nan > upper` are both `False` and the membership checks fell through to `True`: `nan in Bounds(1, 5)` and `nan in a_bounds_set` both reported the value as contained. For metric bounds that is a real hazard — a `NaN` sample would be silently treated as within range. Guard both `Bounds.__contains__` and `BoundsSet.__contains__` with `math.isnan()` so `NaN` is never contained, matching how `None` is already rejected. `math` is already imported for the bisect key. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

The property's docstring promises normalized, merged bounds, but it only delivered that for a `BoundsSet` (whose `bounds` are already normalized). For an `InvalidBoundsSet` — whose `bounds` are the raw, unmerged wire data — it returned the valid entries unmerged, so the same property behaved inconsistently depending on the arm of the union. Run the valid entries through `BoundsSet` in both cases, so the deprecated property always returns the merged, normalized bounds it documents, regardless of whether the underlying set is valid. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

PEP 484's numeric tower makes `int` assignable to any `float`-annotated parameter or field, even under `mypy --strict`, while at runtime `isinstance(1, float)` is `False`. Any field annotated `float` can therefore silently store an `int`, and code unwrapping it with `match … case float():` falls through to `assert_never()`, calls to `float`-only methods like `hex()` crash, and `type()`-based dispatch misbehaves — despite everything type-checking cleanly. There is no clean fix in current Python (see the discussion in frequenz-floss#250): runtime coercion at ingress was prototyped and measured ~2.3× slower on hot-path types, structural `Protocol` tricks don't close the widened variable and `Sequence` covariance holes, and narrowing match arms one by one leaves the annotation lying. So the decision is to stop lying instead: annotate every such value as `float | int`, which is what PEP 484 actually admits, at zero runtime cost. It is also forward-compatible with the typing-council proposal to make `float` mean `float | int` (python/typing-council#46). Add the `FloatInt` alias as the canonical spelling of that union. The alias gives the workaround a single documented home — explaining the trap, the `bool ⊂ int` leak. Follow-up commits migrate the existing `float` annotations to it. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

The protobuf message field is required, so the string will always come. Adding `None` as a possibility in the type system forces user code to deal with `None`, and adds an ambiguity between `None` and `""`. So, as we did with other `name` or similar string fields, it is better to just leave it as a pure `str` and let users deal with the empty string more easily, without the need to special-case `None`. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Introduce a small frozen dataclass pairing the protobuf `category_specific_info` variant name (`kind`) with the leftover fields that this client version did not translate into typed attributes. An electrical component may carry category-specific info on the wire. Known fields are decoded into typed attributes on the concrete component, but anything left over — because the category is unrecognized, or because a newer API version added fields this client doesn't know yet — was either dropped or kept as a bare `dict` with no indication of which variant it came from. This type gives that leftover data a documented home together with the variant name needed to interpret it. It is hashable like the components that will carry it. The leftover `fields` mapping may hold arbitrary, possibly unhashable values, and folding them into the hash — even through `repr()` — is unsafe: values that compare equal can hash or repr differently (e.g. `1 == 1.0 == True`), which would break the invariant that equal objects hash equally. So `fields` is excluded from the hash and instances hash on `kind` alone, mirroring `metric_config_bounds` on the component. Collisions between instances that share a `kind` but differ in `fields` are then possible, but harmless and highly unlikely in practice. A follow-up commit adopts it as the `category_specific_info` field on every electrical component. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Adopt `CategorySpecificInfo | None` as the `category_specific_info` field on `ElectricalComponent`, replacing the bare `Mapping[str, Any]` that only the mismatched component ever populated. The protobuf `category_specific_info` oneof is now decoded once into its `kind` plus a dict of its fields, then each concrete component drops the fields it translated into typed attributes (e.g. a battery's `type`), keeping the `kind` and any leftover. The result: * recognized components keep the variant `kind` with an empty leftover; * `UnrecognizedElectricalComponent` now preserves the carried info instead of dropping it — previously the escape hatch was empty for the very class that needs it most; * `MismatchedCategoryElectricalComponent` keeps the full info (nothing was translated), recording exactly which variant disagreed with the category; * components carrying no variant get `None`. Because `CategorySpecificInfo` is hashable, the field no longer needs `hash=False` and now participates in the component hash like the other fields. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Replace the `{id}<{Class}>{name}` rendering with `{id}:{name}:{Class}`, and override `__str__` on the problematic subclasses so the raw wire value that makes them problematic stays visible in the marker: * recognized components render as `CID42:bat1:LiIonBattery` (the name is always literal, so an empty name reads as `CID42::LiIonBattery`) * `UnrecognizedBattery`/`UnrecognizedEvCharger`/`UnrecognizedInverter` render as `CID42:bat1:Battery:type=99` — the hardcoded base label plus the raw type * `UnrecognizedElectricalComponent` renders as `CID42:comp1:category=999` — the raw, unrecognized category * `MismatchedCategoryElectricalComponent` renders as `CID42:comp1:mismatched:category=5:kind=inverter` — the declared category (as its enum name) against the carried variant kind. Each subclass overrides `__str__` in full rather than sharing a `__str__` hook on the base: the format is a single short f-string, the tests pin it down, and the flat overrides read better than the indirection a hook would need. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

`MismatchedCategoryElectricalComponent.__str__` could only show the declared category as its raw int (`category=5`), because resolving the name would mean using the deprecated `ElectricalComponentCategory` wrapper enum. Store a `category_name` alongside the raw `category`, resolved by the converter from the protobuf enum descriptor (which is not deprecated) with the long `ELECTRICAL_COMPONENT_CATEGORY_` prefix stripped, so `__str__` renders the short, readable name: `CID42:comp1:mismatched:category=BATTERY:kind=inverter` Only the mismatched component needs it: its declared category is a recognized value that its class name hides. Unrecognized components have no name to show (that is what makes them unrecognized), and every other component encodes its category in its class name. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Copilot AI review requested due to automatic review settings July 21, 2026 12:41

Copilot started reviewing on behalf of llucax July 21, 2026 12:42 View session

llucax force-pushed the comp-specific-info branch from 996ca26 to a9bce0f Compare July 21, 2026 12:46

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 21, 2026 12:47

Copilot started reviewing on behalf of llucax July 21, 2026 12:48 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Comment thread src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py

llucax mentioned this pull request Jul 21, 2026

Improve exception messages #258

Draft

llucax force-pushed the comp-specific-info branch 3 times, most recently from 37f4913 to 4b0aad6 Compare July 21, 2026 14:48

llucax added 13 commits July 22, 2026 10:52

llucax added 24 commits July 22, 2026 10:53

Update release notes

c217991

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Update release notes

9f45805

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Use FloatInt in metric bounds

4d28c94

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Use FloatInt in metric samples

73c0e20

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Use FloatInt in Location

97e356d

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Use FloatInt in PowerTransformer

8ee50ec

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Update release notes

b36dd75

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

llucax force-pushed the comp-specific-info branch from 4b0aad6 to 14739e2 Compare July 22, 2026 09:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add a new `CategorySpecificInfo` wrapper#257

Add a new `CategorySpecificInfo` wrapper#257
llucax wants to merge 37 commits into
frequenz-floss:v0.x.xfrom
llucax:comp-specific-info

llucax commented Jul 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

llucax commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

llucax commented Jul 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

llucax commented Jul 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants