Improve exception messages

RELEASE_NOTES.md

@@ -52,6 +52,38 @@
 
     A well-formed `DeliveryArea` has a non-empty `code` and a specified (non-`UNSPECIFIED`) `code_type`. Constructing one with invalid data currently emits a `DeprecationWarning`; a future release will replace the warning with a hard `ValueError`. To opt into the upcoming behavior right now, pass `_raise_on_invalid=True` to the constructor. Prefer `delivery_area_from_proto2` to load delivery areas from the wire — malformed messages become `InvalidDeliveryArea` instances instead.
 
+* `frequenz.client.common.metrics.proto.v1alpha8.bounds_from_proto` is now deprecated; use `bounds_from_proto2` instead.
+
+    The new converter returns `Bounds | InvalidBounds` and surfaces malformed wire data at the type level rather than raising a `ValueError` when `lower > upper`. The old converter continues to work but emits a `DeprecationWarning`.
+
+* `frequenz.client.common.metrics.proto.v1alpha8.bounds_from_proto_with_issues` is now deprecated with no direct replacement.
+
+    Validity is now encoded in the return type of `bounds_from_proto2` (`Bounds | InvalidBounds`), so callers should inspect the returned type instead of collecting issue strings via a side channel. The old converter continues to work but emits a `DeprecationWarning`.
+
+* `frequenz.client.common.metrics.Bounds.__str__` now renders as `[lower,upper]` (no space after the comma) to match the compact format used by `Lifetime` and to compose cleanly with the `<invalid:...>` marker on `InvalidBounds`.
+
+* Several `__str__` representations were standardized around the `<invalid:VALUE>` marker, so a `grep '<invalid:'` over logs finds every invariant violation regardless of which type produced it:
+
+    * `frequenz.client.common.metrics.MetricConnection.__str__` renders as `{name}:{category}` (with `name` possibly empty); known categories render as their member name, the unspecified category renders as `cat=<invalid:0>`, and unknown non-zero categories render as `cat=<int>`.
+    * `frequenz.client.common.metrics.MetricSample` gained a compact `__str__` (`metric=value`, plus `@connection` when a connection is set) instead of falling back to the dataclass `repr`.
+    * `UnrecognizedElectricalComponent`, `MismatchedCategoryElectricalComponent`, `UnrecognizedBattery`, `UnrecognizedEvCharger` and `UnrecognizedInverter` now expose their raw wire `category` / `type` in `__str__` (e.g. `CID1:comp1:Inverter:type=99`), instead of hiding it behind the class name alone. These values are merely unrecognized (forward-compatible), not invariant violations, so they use a plain `:field=value` detail rather than the `<invalid:...>` marker.
+
+* `frequenz.client.common.metrics.MetricSample.bounds` is now deprecated; use `bounds_set` instead.
+
+    The field type changed from `list[Bounds]` to `BoundsSet | InvalidBoundsSet` (see New Features). Reads and construction remain backward compatible: passing the `bounds=` keyword argument still works (it builds a `BoundsSet` and emits a `DeprecationWarning`), and reading `MetricSample.bounds` still returns the valid `Bounds` as a `list` (also emitting a `DeprecationWarning`). The compatibility property returns only the valid, normalized bounds, so it may differ from the raw wire list when bounds overlapped or touched.
+
+* `frequenz.client.common.metrics.proto.v1alpha8.metric_sample_from_proto_with_issues` no longer drops invalid bounds or reports them as a major issue.
+
+    Malformed bounds are now preserved in the returned `MetricSample.bounds_set` as an `InvalidBoundsSet` (validity is encoded in the type), so the previous "bounds for ... is invalid, ignoring these bounds" major issue is no longer produced.
+
+* `float`-typed fields and accessors are now annotated with the new `FloatInt` (`float | int`) type alias (see New Features), to be honest about what PEP 484's numeric tower actually admits. These symbols are affected:
+
+    * `frequenz.client.common.metrics.AggregatedMetricValue`: the `avg`, `min`, `max` and `raw` fields.
+    * `frequenz.client.common.metrics.MetricSample`: the `value` field and the `as_single_value()` return type.
+    * `frequenz.client.common.metrics.Bounds`: the `lower` and `upper` fields (shared with the new `BaseBounds` / `InvalidBounds` hierarchy).
+
+    Runtime behavior is completely unchanged: these fields could always end up storing `int` values (`x: float = 1` is legal even under `mypy --strict`), the annotations just didn't admit it. Reads that assign to `float`-typed destinations or do plain arithmetic keep type-checking as before. However, code that pattern-matches these values with a bare `case float():` arm — a latent runtime crash, since `isinstance(1, float)` is `False` — will now be flagged as non-exhaustive by strict type checkers and should be widened to `case float() | int():`, and calling `float`-only methods (e.g. `hex()`) on them now requires an explicit `float(...)` conversion.
+
 ## New Features
 
 * Added 4 new electrical component classes for categories that previously collapsed into `UnrecognizedElectricalComponent`:
@@ -81,6 +113,8 @@
     * `frequenz.client.common.grid.DeliveryArea.get_code_type()`
     * `frequenz.client.common.metrics.MetricConnection.get_category()`
     * `frequenz.client.common.metrics.MetricSample.get_metric()`
+    * `frequenz.client.common.metrics.MetricSample.get_bounds_set()`
+    * `frequenz.client.common.microgrid.electrical_components.ElectricalComponent.get_metric_config_bounds()`
 
 * Added new delivery-area class hierarchy:
 
@@ -92,6 +126,20 @@
 
 * Added a new `frequenz.client.common.microgrid.Lifetime` type together with the `frequenz.client.common.microgrid.proto.v1alpha8.lifetime_from_proto` conversion function.
 
+* Added `frequenz.client.common.metrics.proto.v1alpha8.bounds_from_proto2` returning `Bounds | InvalidBounds`. This is the replacement for the now-deprecated `bounds_from_proto`.
+
+* `frequenz.client.common.metrics.Bounds` gained containment check capabilities:
+
+    * `value in bounds` (`__contains__`) tests membership, inclusive on both ends, with a `None` bound meaning unbounded in that direction.
+    * `bool(bounds)` and `bounds.is_bounded()` report whether the bounds restrict anything; a fully unbounded `Bounds()` is falsy.
+
+* Added a new bounds-set class hierarchy:
+
+    * `frequenz.client.common.metrics.BoundsSet` — a normalized union of `Bounds` with an efficient `value in bounds_set` membership test. Overlapping and touching bounds are merged on construction, and the empty set is the unbounded set (it contains every value and is falsy).
+    * `frequenz.client.common.metrics.InvalidBoundsSet` — a set built from bounds that included at least one `InvalidBounds`; it preserves all the raw bounds unmerged and provides no membership test.
+
+* Added a new `frequenz.client.common.metrics.MetricSample.bounds_set` field, typed `BoundsSet | InvalidBoundsSet`, replacing the deprecated `bounds` list (see Upgrading). Malformed wire bounds are preserved as an `InvalidBoundsSet` instead of being dropped. Use `get_bounds_set()` to resolve it to a valid `BoundsSet` or a clear `InvalidBoundsSetError`.
+
 * 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.
@@ -100,8 +148,15 @@
 
     The class of a component is its identity; components don't carry category or type attributes. The only exceptions are the error-recovery classes `UnrecognizedElectricalComponent` and `MismatchedCategoryElectricalComponent` (with a raw protobuf `category` value) and `UnrecognizedBattery`, `UnrecognizedInverter` and `UnrecognizedEvCharger` (with a raw protobuf `type` value), which preserve the raw protobuf values received from the protocol version used to load them.
 
+    `ElectricalComponent.metric_config_bounds` is typed `Mapping[Metric | int, Bounds | InvalidBounds]`: malformed wire entries are preserved as `InvalidBounds` instead of being silently dropped, and entries that named a metric but carried no `config_bounds` field load as an unbounded `Bounds()` (that is the explicit wire encoding of an unbounded metric, since protobuf never serializes an all-defaults `Bounds` message). Use `get_metric_config_bounds()` to resolve an entry to a valid `Bounds` or a clear `InvalidBoundsError`; it mimics `dict.get()`, returning an unbounded `Bounds()` (or a caller-supplied `default`) for absent metrics.
+
 * 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 `frequenz.client.common.FloatInt`, a type alias for `float | int`.
+
+    PEP 484's numeric tower makes `int` assignable wherever `float` is annotated, even under `mypy --strict`, while at runtime `isinstance(1, float)` is `False` — so a plain `float` annotation silently admits values that crash `match … case float():` arms and `float`-only methods like `hex()`. The library now spells such annotations `FloatInt` instead of lying (see Upgrading); the alias docstring documents the trap in detail, including the inherent `bool ⊂ int` leak. New numeric fields (`Location` latitudes/longitudes, `PowerTransformer` voltages, bounds and bounds sets) use it as well. Values loaded from protobuf are unaffected in practice, as the wire always delivers real `float`s.
+
 ## Bug Fixes
 
 * Fixed `EnumParityTest` so protobuf values whose Python member name exists with a different number fail parity checks instead of being treated as unmirrored protobuf values.
+* Fixed potential unexpected exceptions due to type-checking accepting `int` for code annotated to only accept `float`. Fixes #250.

src/frequenz/client/common/__init__.py

@@ -10,9 +10,11 @@
     UnrecognizedEnumValueError,
     UnspecifiedEnumValueError,
 )
+from ._float import FloatInt
 
 __all__ = [
     "ClientCommonError",
+    "FloatInt",
     "InvalidAttributeError",
     "MissingFieldError",
     "UnrecognizedEnumValueError",

src/frequenz/client/common/_exception.py

@@ -70,7 +70,7 @@ def __init__(
             (
                 message
                 if message is not None
-                else f"unrecognized enum value {value!r} for attribute {attr_name!r} in {instance}"
+                else f"unrecognized enum value {value} for attribute {attr_name!r} in {instance}"
             ),
         )
 

src/frequenz/client/common/_float.py

@@ -0,0 +1,50 @@
+# License: MIT
+# Copyright © 2026 Frequenz Energy-as-a-Service GmbH
+
+"""Honest type alias for floating-point values."""
+
+from typing import TypeAlias
+
+FloatInt: TypeAlias = float | int
+"""A `float` that may actually be an `int` at runtime.
+
+[PEP 484's numeric tower](https://peps.python.org/pep-0484/#the-numeric-tower)
+makes `int` assignable to any `float`-annotated parameter or field, so a plain
+`float` annotation is a lie: type checkers (even `mypy --strict`) happily
+accept `int` values, but `isinstance(1, float)` is `False` at runtime. That
+breaks `match … case float():` arms (an `int` value falls through to
+`assert_never()`), calls to `float`-only methods like `hex()`, and any other
+code dispatching on the concrete runtime type.
+
+This library instead annotates such values as `FloatInt`, making the
+heterogeneity explicit: type checkers will push code reading these values to
+handle both branches, typically by matching with `case float() | int():`. See
+[issue #250](https://github.com/frequenz-floss/frequenz-client-common-python/issues/250)
+for the full analysis and the alternatives that were rejected.
+
+Danger:
+    `bool` is a subclass of `int`, so `True` and `False` also satisfy this
+    alias. This is inherent to Python's type system and not guarded against.
+
+Example:
+    ```python
+    from typing import assert_never
+
+    from frequenz.client.common import FloatInt
+
+
+    def describe(value: FloatInt | None) -> str:
+        match value:
+            case float() | int():
+                return f"number {value}"
+            case None:
+                return "nothing"
+            case unexpected:
+                assert_never(unexpected)
+
+
+    assert describe(1) == "number 1"
+    assert describe(1.5) == "number 1.5"
+    assert describe(None) == "nothing"
+    ```
+"""

src/frequenz/client/common/grid/_delivery_area.py

@@ -274,7 +274,7 @@ def __init__(
         """The invalid delivery area instance that caused this error."""
 
         message = (
-            f"invalid delivery area {delivery_area!r} for attribute {attr_name!r} in {instance}"
+            f"invalid delivery area {delivery_area} for attribute {attr_name!r} in {instance}"
             if message is None
             else message
         )

src/frequenz/client/common/metrics/__init__.py

@@ -3,7 +3,15 @@
 
 """Metrics definitions."""
 
-from ._bounds import Bounds
+from ._bounds import (
+    BaseBounds,
+    Bounds,
+    BoundsSet,
+    InvalidBounds,
+    InvalidBoundsError,
+    InvalidBoundsSet,
+    InvalidBoundsSetError,
+)
 from ._metric import Metric
 from ._sample import (
     AggregatedMetricValue,
@@ -16,7 +24,13 @@
 __all__ = [
     "AggregatedMetricValue",
     "AggregationMethod",
+    "BaseBounds",
     "Bounds",
+    "BoundsSet",
+    "InvalidBounds",
+    "InvalidBoundsError",
+    "InvalidBoundsSet",
+    "InvalidBoundsSetError",
     "Metric",
     "MetricConnection",
     "MetricConnectionCategory",