Improve exception messages by llucax · Pull Request #258 · frequenz-floss/frequenz-client-common-python

llucax · 2026-07-21T12:48:40Z

Use {value} instead of {value!r} for errors printing the value, so they use a shorter, less verbose and redundant representation of invalid data, that now explicitly use the <invalid:...> pattern.

Copilot

Pull request overview

This PR broadens the library’s “compact invalid marker” approach across multiple domains (metrics, microgrid electrical components, and location), introducing new helper types and updating conversions/tests to preserve malformed wire data explicitly rather than dropping it or only reporting it via side-channel issues.

Changes:

Introduce FloatInt (float | int) and update several previously-float annotations to be runtime-honest about PEP 484’s numeric tower.
Add a bounds type hierarchy (Bounds/InvalidBounds, BoundsSet/InvalidBoundsSet) plus new/updated accessors and proto conversion behavior (including deprecations).
Standardize/extend string representations (and related tests) to be more compact and to surface raw unrecognized values.

Reviewed changes

Copilot reviewed 57 out of 57 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`tests/types/_location/test_location.py`	Expands latitude/longitude test coverage to include `int` and `FloatInt` typing.
`tests/types/_location/test_invalid_longitude.py`	Adds coverage that invalid longitude can preserve an `int` value.
`tests/types/_location/test_invalid_latitude.py`	Adds coverage that invalid latitude can preserve an `int` value.
`tests/test_float.py`	Adds tests for the new `FloatInt` alias.
`tests/microgrid/proto/v1alpha8/test_microgrid.py`	Removes log-expectation assertions from microgrid proto conversion tests.
`tests/microgrid/electrical_components/test_problematic.py`	Adds `__str__` expectations for problematic/unrecognized component types.
`tests/microgrid/electrical_components/test_power_transformer.py`	Updates transformer voltage tests to accept `FloatInt` inputs.
`tests/microgrid/electrical_components/test_inverter.py`	Adds `__str__` expectation for `UnrecognizedInverter`.
`tests/microgrid/electrical_components/test_ev_charger.py`	Adds `__str__` expectation for `UnrecognizedEvCharger`.
`tests/microgrid/electrical_components/test_electrical_component_base.py`	Updates component base tests for `category_specific_info` and adds metric-bounds accessor tests.
`tests/microgrid/electrical_components/test_category_specific_info.py`	Adds tests for new `CategorySpecificInfo`.
`tests/microgrid/electrical_components/test_battery.py`	Adds `__str__` expectation for `UnrecognizedBattery`.
`tests/microgrid/electrical_components/proto/v1alpha8/test_raw_storage.py`	Extends proto tests around preserving category-specific info.
`tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_simple.py`	Updates proto tests to use `CategorySpecificInfo` objects.
`tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py`	Updates metric-config-bounds parsing expectations, incl. invalid bounds preservation.
`tests/microgrid/electrical_components/proto/v1alpha8/conftest.py`	Adjusts fixtures for `category_specific_info=None` and removes old metadata assertion.
`tests/microgrid/_lifetime/test_invalid_lifetime_error.py`	Updates expected lifetime error string formatting.
`tests/metrics/test_sample_metric_sample.py`	Migrates tests to `bounds_set` + adds string/accessor/deprecation-path coverage.
`tests/metrics/test_sample_metric_connection.py`	Updates expected `MetricConnection.__str__` formatting and name default behavior.
`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 suite under `_bounds/`).
`tests/metrics/proto/v1alpha8/test_sample_metric_sample.py`	Updates proto sample tests to use `bounds_set` and preserve invalid bounds.
`tests/metrics/proto/v1alpha8/test_sample_metric_connection.py`	Updates proto connection tests for empty-name behavior.
`tests/metrics/proto/v1alpha8/test_bounds.py`	Adds coverage for new `bounds_from_proto2` and deprecation warnings on old APIs.
`tests/metrics/_bounds/test_invalid_bounds.py`	Adds tests for `InvalidBounds`.
`tests/metrics/_bounds/test_invalid_bounds_set.py`	Adds tests for `InvalidBoundsSet`.
`tests/metrics/_bounds/test_invalid_bounds_set_error.py`	Adds tests for `InvalidBoundsSetError`.
`tests/metrics/_bounds/test_invalid_bounds_error.py`	Adds tests for `InvalidBoundsError`.
`tests/metrics/_bounds/test_bounds.py`	Adds updated `Bounds` tests (string form, containment, truthiness).
`tests/metrics/_bounds/test_bounds_set.py`	Adds tests for `BoundsSet` normalization/containment semantics.
`tests/metrics/_bounds/test_base_bounds.py`	Adds test that `BaseBounds` cannot be instantiated.
`tests/metrics/_bounds/__init__.py`	Adds package marker for new bounds test module.
`tests/grid/_delivery_area/test_invalid_delivery_area_error.py`	Updates expected delivery-area error message formatting.
`src/frequenz/client/common/types/_location.py`	Switches location numeric fields/accessors/errors to `FloatInt` and adjusts messages/docs.
`src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py`	Removes issue-collection/logging and simplifies `_active` assignment.
`src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py`	Preserves invalid bounds (`InvalidBounds`), introduces `CategorySpecificInfo`, and updates parsing helpers.
`src/frequenz/client/common/microgrid/electrical_components/_problematic.py`	Adds/updates `__str__` for problematic component types and enriches mismatch info.
`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`	Updates metric bounds typing, adds `get_metric_config_bounds()`, and standardizes `__str__`.
`src/frequenz/client/common/microgrid/electrical_components/_category_specific_info.py`	Adds new `CategorySpecificInfo` type.
`src/frequenz/client/common/microgrid/electrical_components/_battery.py`	Adds `__str__` for `UnrecognizedBattery`.
`src/frequenz/client/common/microgrid/electrical_components/__init__.py`	Exports new symbols (`CategorySpecificInfo`, `DefaultT`).
`src/frequenz/client/common/microgrid/_microgrid.py`	Updates error message formatting for unrecognized status.
`src/frequenz/client/common/microgrid/_lifetime.py`	Updates error message formatting to use `{value}` vs `{value!r}`.
`src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py`	Switches bounds parsing to `bounds_from_proto2` and returns `BoundsSet
`src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py`	Deprecates old bounds converters and adds `bounds_from_proto2`.
`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`, updates `MetricConnection.__str__`, adds new accessors.
`src/frequenz/client/common/metrics/_bounds.py`	Adds bounds/bounds-set type hierarchy, containment semantics, and invalid-preserving error types.
`src/frequenz/client/common/metrics/__init__.py`	Exports new bounds-related public API.
`src/frequenz/client/common/grid/_delivery_area.py`	Updates delivery-area error message formatting to use `{value}` vs `{value!r}`.
`src/frequenz/client/common/_float.py`	Adds the new public `FloatInt` type alias and its rationale.
`src/frequenz/client/common/_exception.py`	Updates `UnrecognizedEnumValueError` message formatting to use `{value}` vs `{value!r}`.
`src/frequenz/client/common/__init__.py`	Exports `FloatInt`.
`RELEASE_NOTES.md`	Documents deprecations/new features and string-format changes.

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

llucax · 2026-07-21T12:59:37Z

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

Render `<invalid:0>` 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 * `<CATEGORY=...>` -> bare `...` for enum values and `:cat=...` for `int` values for compactness Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

`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 `<invalid:0>`, 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 <luca-frequenz@llucax.com>

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. `<invalid:[10.0,-10.0]>` instead of `InvalidBounds(lower=10.0, upper=-10.0)` — which surfaces the `<invalid:...>` 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 <luca-frequenz@llucax.com>

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

llucax · 2026-08-14T09:39:18Z

Ready for review.

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

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

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Comment thread tests/test_float.py

Comment thread RELEASE_NOTES.md Outdated

Comment thread RELEASE_NOTES.md

llucax force-pushed the no-repr-value-instance branch from fb62cc0 to 61061a6 Compare July 21, 2026 12:58

llucax force-pushed the no-repr-value-instance branch 5 times, most recently from 71b58c0 to 12fb771 Compare July 22, 2026 13:28

llucax force-pushed the no-repr-value-instance branch 3 times, most recently from a51c57e to c90ff69 Compare August 13, 2026 13:03

llucax added 4 commits August 14, 2026 11:35

Update release notes

85ba572

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

llucax force-pushed the no-repr-value-instance branch from c90ff69 to 85ba572 Compare August 14, 2026 09:37

llucax marked this pull request as ready for review August 14, 2026 09:38

llucax requested a review from a team as a code owner August 14, 2026 09:38

llucax requested review from ela-kotulska-frequenz and removed request for a team August 14, 2026 09:38

llucax self-assigned this Aug 14, 2026

llucax enabled auto-merge August 14, 2026 09:38

llucax requested review from Marenz and tiyash-basu-frequenz August 14, 2026 09:39

tiyash-basu-frequenz approved these changes Aug 14, 2026

View reviewed changes

llucax added this pull request to the merge queue Aug 14, 2026

Merged via the queue into frequenz-floss:v0.x.x with commit 0dee433 Aug 14, 2026
9 checks passed

llucax deleted the no-repr-value-instance branch August 14, 2026 11:49

Navigation Menu

Uh oh!

Improve exception messages - #258

Improve exception messages#258
llucax merged 4 commits into
frequenz-floss:v0.x.xfrom
llucax:no-repr-value-instance

llucax commented Jul 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

llucax commented Jul 21, 2026

Uh oh!

llucax commented Aug 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 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!

llucax commented Jul 21, 2026

Uh oh!

llucax commented Aug 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants