From ef6d7f158a08958532a8c6be399637edb69888bd Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 20 Jul 2026 12:43:29 +0000 Subject: [PATCH 1/4] Standardize the invalid marker in `MetricConnection.__str__` Render `` when the category is unspecified (the raw `0` or the deprecated `UNSPECIFIED` member). Unknown non-zero ints keep rendering bare: they are forward-compatible values this client version doesn't know yet, not invariant violations. The format is slightly changed to make it more familiar with other `__str__` representations: * `None`/`""` are not merged and kept literal * `` -> bare `...` for enum values and `:cat=...` for `int` values for compactness Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/metrics/_sample.py | 20 +++++++++++-------- .../metrics/test_sample_metric_connection.py | 20 +++++++++++++++---- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/frequenz/client/common/metrics/_sample.py b/src/frequenz/client/common/metrics/_sample.py index a22fc409..1a69393f 100644 --- a/src/frequenz/client/common/metrics/_sample.py +++ b/src/frequenz/client/common/metrics/_sample.py @@ -128,14 +128,18 @@ class MetricConnection: def __str__(self) -> str: """Return a string representation of this connection.""" - category_name = ( - str(self.category) - if isinstance(self.category, int) - else f"" - ) - if self.name: - return f"{category_name}({self.name})" - return category_name + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + match self.category: + case 0 | MetricConnectionCategory.UNSPECIFIED: + category_name = "cat=" + case MetricConnectionCategory() as category: + category_name = category.name + case int() as category: + category_name = f"cat={category}" + case unexpected: + assert_never(unexpected) + return f"{self.name}:{category_name}" def get_category(self) -> MetricConnectionCategory: """Return the connection category as a known enum member. diff --git a/tests/metrics/test_sample_metric_connection.py b/tests/metrics/test_sample_metric_connection.py index d6888c9a..1b2cfe1b 100644 --- a/tests/metrics/test_sample_metric_connection.py +++ b/tests/metrics/test_sample_metric_connection.py @@ -18,27 +18,39 @@ pytest.param( MetricConnectionCategory.BATTERY, "", - "", + ":BATTERY", id="enum_category_empty_name", ), pytest.param( MetricConnectionCategory.PV, "dc_pv_0", - "(dc_pv_0)", + "dc_pv_0:PV", id="enum_category_with_name", ), pytest.param( 999, "", - "999", + ":cat=999", id="int_category_empty_name", ), pytest.param( 999, "unknown_connection", - "999(unknown_connection)", + "unknown_connection:cat=999", id="int_category_with_name", ), + pytest.param( + 0, + "", + ":cat=", + id="unspecified_int_empty_name", + ), + pytest.param( + 0, + "conn", + "conn:cat=", + id="unspecified_int_with_name", + ), ], ) def test_str_representation( From ec4a01e5c7e0a7c8fc3d932fe04b42d61f0be9b9 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 20 Jul 2026 12:51:00 +0000 Subject: [PATCH 2/4] Add `MetricSample.__str__` `MetricSample` had no `__str__`, so it fell back to the verbose dataclass `__repr__`. Add a compact one rendering `{metric}={value}`, with `@{connection}` appended when a connection is set. The metric follows the same convention as `MetricConnection.__str__`: a known member renders as its name, the unspecified sentinel (`0` / `UNSPECIFIED`) as ``, and an unknown int bare. `MetricSample` is the `instance` reported by `get_metric()` (`Unrecognized` / `UnspecifiedEnumValueError`) and `get_bounds_set()` (`InvalidBoundsSetError`); without a `__str__` those messages embedded the whole dataclass repr. A short representation keeps the errors readable. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/metrics/_sample.py | 18 ++++++ tests/metrics/test_sample_metric_sample.py | 61 +++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/src/frequenz/client/common/metrics/_sample.py b/src/frequenz/client/common/metrics/_sample.py index 1a69393f..0d774a04 100644 --- a/src/frequenz/client/common/metrics/_sample.py +++ b/src/frequenz/client/common/metrics/_sample.py @@ -290,6 +290,24 @@ def __init__( object.__setattr__(self, "bounds_set", bounds_set) object.__setattr__(self, "connection", connection) + def __str__(self) -> str: + """Return a compact string representation of this sample.""" + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=DeprecationWarning) + match self.metric: + case 0 | Metric.UNSPECIFIED: + metric = "" + case Metric() as known: + metric = known.name + case int() as unknown: + metric = str(unknown) + case unexpected: + assert_never(unexpected) + sample = f"{metric}={self.value}" + if self.connection is not None: + sample = f"{sample}@{self.connection}" + return sample + @property @deprecated("`MetricSample.bounds` is deprecated; use `bounds_set` instead.") def bounds(self) -> list[Bounds]: diff --git a/tests/metrics/test_sample_metric_sample.py b/tests/metrics/test_sample_metric_sample.py index 4d255b73..69f0defd 100644 --- a/tests/metrics/test_sample_metric_sample.py +++ b/tests/metrics/test_sample_metric_sample.py @@ -22,6 +22,7 @@ InvalidBoundsSetError, Metric, MetricConnection, + MetricConnectionCategory, MetricSample, ) @@ -78,6 +79,66 @@ def test_creation( assert sample.connection == connection +@pytest.mark.parametrize( + "metric, value, connection, expected", + [ + pytest.param( + Metric.AC_POWER_ACTIVE, + 5.0, + None, + "AC_POWER_ACTIVE=5.0", + id="known_metric", + ), + pytest.param( + Metric.AC_POWER_ACTIVE, + 5.0, + MetricConnection( + category=MetricConnectionCategory.BATTERY, name="dc_battery_0" + ), + "AC_POWER_ACTIVE=5.0@dc_battery_0:BATTERY", + id="with_connection", + ), + pytest.param( + 0, + None, + None, + "=None", + id="unspecified_metric", + ), + pytest.param( + 99999, + 42, + None, + "99999=42", + id="unrecognized_metric", + ), + pytest.param( + Metric.AC_POWER_ACTIVE, + AggregatedMetricValue(avg=5.0, min=1.0, max=10.0, raw=[1.0, 5.0, 10.0]), + None, + "AC_POWER_ACTIVE=avg:5.0", + id="aggregated_value", + ), + ], +) +def test_str( + now: datetime, + metric: Metric | int, + value: FloatInt | AggregatedMetricValue | None, + connection: MetricConnection | None, + expected: str, +) -> None: + """`MetricSample.__str__` renders a compact `metric=value` summary.""" + sample = MetricSample( + sample_time=now, + metric=metric, + value=value, + bounds_set=BoundsSet(), + connection=connection, + ) + assert str(sample) == expected + + @pytest.mark.parametrize( "value, method_results", [ From 88c70508b3b32692b7f002277c2bf0a605460fa8 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 20 Jul 2026 12:54:28 +0000 Subject: [PATCH 3/4] Use `str` instead of `repr` for values error messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default (and custom) messages of the accessor errors interpolated the offending value with `!r`. Switch the non-string values to plain `{value}` so wrapper values render through their compact `__str__` — e.g. `` instead of `InvalidBounds(lower=10.0, upper=-10.0)` — which surfaces the `` markers in the message and keeps it readable. Covered: `UnrecognizedEnumValueError`, `InvalidLatitudeError`, `InvalidLongitudeError`, `InvalidLifetimeError`, `InvalidBoundsError`, `InvalidBoundsSetError` and `InvalidDeliveryAreaError`, plus the custom messages raised by `ElectricalComponent.provides_telemetry()`, `accepts_control()` and `get_metric_config_bounds()`, and `Microgrid.is_active()`. String values keep `!r`: an invalid `country_code` may carry surprising characters, and quoting keeps them visible. The `attr_name` likewise keeps `!r`, the idiomatic way to show an attribute name. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/_exception.py | 2 +- src/frequenz/client/common/grid/_delivery_area.py | 2 +- src/frequenz/client/common/metrics/_bounds.py | 4 ++-- src/frequenz/client/common/microgrid/_lifetime.py | 2 +- src/frequenz/client/common/microgrid/_microgrid.py | 2 +- .../electrical_components/_electrical_component.py | 6 +++--- src/frequenz/client/common/types/_location.py | 4 ++-- .../grid/_delivery_area/test_invalid_delivery_area_error.py | 2 +- tests/metrics/_bounds/test_invalid_bounds_error.py | 2 +- tests/metrics/_bounds/test_invalid_bounds_set_error.py | 2 +- tests/microgrid/_lifetime/test_invalid_lifetime_error.py | 2 +- 11 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/frequenz/client/common/_exception.py b/src/frequenz/client/common/_exception.py index dd7317b4..54313175 100644 --- a/src/frequenz/client/common/_exception.py +++ b/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}" ), ) diff --git a/src/frequenz/client/common/grid/_delivery_area.py b/src/frequenz/client/common/grid/_delivery_area.py index 578476ae..c51ba40e 100644 --- a/src/frequenz/client/common/grid/_delivery_area.py +++ b/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 ) diff --git a/src/frequenz/client/common/metrics/_bounds.py b/src/frequenz/client/common/metrics/_bounds.py index 5b051b1d..8f9fe3f7 100644 --- a/src/frequenz/client/common/metrics/_bounds.py +++ b/src/frequenz/client/common/metrics/_bounds.py @@ -169,7 +169,7 @@ def __init__( ( message if message is not None - else f"invalid bounds {bounds!r} for attribute {attr_name!r} in {instance}" + else f"invalid bounds {bounds} for attribute {attr_name!r} in {instance}" ), ) @@ -419,6 +419,6 @@ def __init__( ( message if message is not None - else f"invalid bounds set {bounds_set!r} for attribute {attr_name!r} in {instance}" + else f"invalid bounds set {bounds_set} for attribute {attr_name!r} in {instance}" ), ) diff --git a/src/frequenz/client/common/microgrid/_lifetime.py b/src/frequenz/client/common/microgrid/_lifetime.py index e28c5736..c35bce31 100644 --- a/src/frequenz/client/common/microgrid/_lifetime.py +++ b/src/frequenz/client/common/microgrid/_lifetime.py @@ -150,6 +150,6 @@ def __init__( ( message if message is not None - else f"invalid lifetime {lifetime!r} for attribute {attr_name!r} in {instance}" + else f"invalid lifetime {lifetime} for attribute {attr_name!r} in {instance}" ), ) diff --git a/src/frequenz/client/common/microgrid/_microgrid.py b/src/frequenz/client/common/microgrid/_microgrid.py index 799c76fc..bac17238 100644 --- a/src/frequenz/client/common/microgrid/_microgrid.py +++ b/src/frequenz/client/common/microgrid/_microgrid.py @@ -117,7 +117,7 @@ def is_active(self) -> bool: self, "_active", value, - f"unrecognized status of microgrid {self}: {value!r}", + f"unrecognized status of microgrid {self}: {value}", ) case unknown: assert_never(unknown) 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 bbde0005..f2680ab1 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -162,7 +162,7 @@ def provides_telemetry(self) -> bool: self, "_provides_telemetry", value, - f"operational mode {value!r} of {self} is not a recognized " + f"operational mode {value} of {self} is not a recognized " "ElectricalComponentOperationalMode; telemetry availability " "is unknown", ) @@ -195,7 +195,7 @@ def accepts_control(self) -> bool: self, "_accepts_control", value, - f"operational mode {value!r} of {self} is not a recognized " + f"operational mode {value} of {self} is not a recognized " "ElectricalComponentOperationalMode; control availability " "is unknown", ) @@ -257,7 +257,7 @@ def get_metric_config_bounds( self, "metric_config_bounds", invalid, - f"invalid bounds {invalid!r} for metric {metric} in {self}", + f"invalid bounds {invalid} for metric {metric} in {self}", ) case Bounds() as valid: return valid diff --git a/src/frequenz/client/common/types/_location.py b/src/frequenz/client/common/types/_location.py index bd946ed8..248066e0 100644 --- a/src/frequenz/client/common/types/_location.py +++ b/src/frequenz/client/common/types/_location.py @@ -44,7 +44,7 @@ def __init__( ( message if message is not None - else f"invalid latitude {value!r} for attribute {attr_name!r} in " + else f"invalid latitude {value} for attribute {attr_name!r} in " f"{instance}; must be in [-90, 90]" ), ) @@ -84,7 +84,7 @@ def __init__( ( message if message is not None - else f"invalid longitude {value!r} for attribute {attr_name!r} in " + else f"invalid longitude {value} for attribute {attr_name!r} in " f"{instance}; must be in [-180, 180]" ), ) diff --git a/tests/grid/_delivery_area/test_invalid_delivery_area_error.py b/tests/grid/_delivery_area/test_invalid_delivery_area_error.py index 9bb7be28..cb1c205d 100644 --- a/tests/grid/_delivery_area/test_invalid_delivery_area_error.py +++ b/tests/grid/_delivery_area/test_invalid_delivery_area_error.py @@ -15,7 +15,7 @@ def test_default_message() -> None: error = InvalidDeliveryAreaError("some-instance", "delivery_area", invalid) assert error.delivery_area is invalid assert ( - "invalid delivery area InvalidDeliveryArea(code='', code_type=0) for " + f"invalid delivery area {invalid} for " "attribute 'delivery_area' in some-instance" == str(error) ) diff --git a/tests/metrics/_bounds/test_invalid_bounds_error.py b/tests/metrics/_bounds/test_invalid_bounds_error.py index cd12e742..80e06499 100644 --- a/tests/metrics/_bounds/test_invalid_bounds_error.py +++ b/tests/metrics/_bounds/test_invalid_bounds_error.py @@ -14,7 +14,7 @@ def test_default_message() -> None: assert error.bounds is invalid assert ( - str(error) == f"invalid bounds {invalid!r} for attribute 'config_bounds' " + str(error) == f"invalid bounds {invalid} for attribute 'config_bounds' " "in some-instance" ) diff --git a/tests/metrics/_bounds/test_invalid_bounds_set_error.py b/tests/metrics/_bounds/test_invalid_bounds_set_error.py index 6f44eac4..d1d62da0 100644 --- a/tests/metrics/_bounds/test_invalid_bounds_set_error.py +++ b/tests/metrics/_bounds/test_invalid_bounds_set_error.py @@ -21,7 +21,7 @@ def test_default_message() -> None: assert error.bounds_set is invalid assert ( - str(error) == f"invalid bounds set {invalid!r} for attribute 'bounds_set' " + str(error) == f"invalid bounds set {invalid} for attribute 'bounds_set' " "in some-instance" ) diff --git a/tests/microgrid/_lifetime/test_invalid_lifetime_error.py b/tests/microgrid/_lifetime/test_invalid_lifetime_error.py index e8e56f75..9a917266 100644 --- a/tests/microgrid/_lifetime/test_invalid_lifetime_error.py +++ b/tests/microgrid/_lifetime/test_invalid_lifetime_error.py @@ -17,7 +17,7 @@ def test_default_message(present: datetime, future: datetime) -> None: assert error.lifetime is invalid assert ( str(error) - == f"invalid lifetime {invalid!r} for attribute 'operational_lifetime' " + == f"invalid lifetime {invalid} for attribute 'operational_lifetime' " "in some-instance" ) From 85ba5728ba1a747108e2b6addf14de0cb7a18a0a Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 20 Jul 2026 12:56:20 +0000 Subject: [PATCH 4/4] Update release notes Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index f37b2067..6f2a78d1 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -62,6 +62,12 @@ * `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 `` marker on `InvalidBounds`. +* Several `__str__` representations were standardized around the `` marker, so a `grep '`, and unknown non-zero categories render as `cat=`. + * `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 `` 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.