diff --git a/AGENTS.md b/AGENTS.md index 9e0b0d53..1fccba4d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,6 +18,7 @@ types never leak into a public signature — they are confined to the `proto/` c . ├── src/frequenz/client/common/ # the library (PEP 420 namespace pkg) ├── tests/ # mirrors src/ layout by domain + proto version +├── docs/{user-guide,client-developer-guide,wrapping-guide}/ # authored guides — the canonical how-to (see WHERE TO LOOK) ├── docs/_scripts/ # mkdocstrings autoapi generation (not prose) ├── noxfile.py # 8 lines; delegates everything to frequenz-repo-config └── site/ # generated docs output — do NOT hand-edit or commit @@ -27,8 +28,10 @@ types never leak into a public signature — they are confined to the `proto/` c | Task | Location | Notes | |------|----------|-------| -| Add/change a public type or enum | `src/...//` | See `src/.../AGENTS.md` for the pattern | -| Add/change protobuf conversion | `src/...//proto//` | Per API namespace; only `v1alpha8` exists | +| Design a wrapper or converter (patterns) | `docs/wrapping-guide/` | Canonical: enums, data types, validity, converters, deprecation. Don't duplicate it in code/AGENTS | +| Consume wrappers / build a client library | `docs/user-guide/`, `docs/client-developer-guide/` | Downstream users + client-lib authors | +| Add/change a public type or enum | `src/...//` | Repo mechanics in `src/.../AGENTS.md`; design in `docs/wrapping-guide/` | +| Add/change protobuf conversion | `src/...//proto//` | Per API namespace (only `v1alpha8`); see `docs/wrapping-guide/conversion-functions.md` | | Shared enum proto helper | `src/.../proto/_enum.py` | `enum_from_proto` used by every enum wrapper | | Reusable enum test scaffold | `src/.../test/enum_parity.py` | `EnumParityTest` base class | | Add tests | `tests//...` | Mirrors src tree; see `tests/AGENTS.md` | @@ -57,7 +60,7 @@ types never leak into a public signature — they are confined to the `proto/` c ```bash python -m pip install -e .[dev] # full dev install nox # all checks (creates own venvs) -nox -R -s pytest -- tests/test_*.py # tests, reuse env +nox -R -s pytest_max -- tests/... # tests (min-deps: pytest_min); reuse env nox -R -s pylint -- ... # lint; nox -R -s mypy -- ... for types pytest # direct run (needs .[dev-pytest]) mkdocs serve # live docs preview diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d11ab596..35bfdee8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -117,27 +117,9 @@ GitHub pages produced for your fork. ## Wrapper conventions -### Field names and docstrings - -The following rules apply when wrapping protobuf messages into idiomatic Python -types: - -1. **Alignment**: By default, wrapper field names match the protobuf field names - unless a clear Pythonic improvement preserves or clarifies semantics. -2. **IDs**: Always keep the `_id` suffix for fields representing identifiers - (e.g., `id`, `microgrid_id`, `enterprise_id`, `source_id`, `destination_id`). -3. **Redundancy**: You may drop a redundant or long entity prefix while keeping - `_id`. For example, `source_electrical_component_id` becomes `source_id`. -4. **Time fields**: Use the `_time` suffix for both protobuf `_time` and - `_timestamp` fields. Never drop the suffix, as bare names like `start` or - `create` can be read as verbs or actions. For example, `create_timestamp` - becomes `create_time` and `start_timestamp` becomes `start_time`. -5. **Values**: You may drop the `_value` suffix inside a class ending in `Value` - when the remaining name remains clear. For example, `avg`, `min`, `max`, and - `raw` in `AggregatedMetricValue`. -6. **Docstrings**: Docstrings may be shorter or more Pythonic than the protobuf - comments, but they must not contradict, narrow, broaden, or operationally - reinterpret the protobuf semantics. +The [Wrapping Guide](https://frequenz-floss.github.io/frequenz-client-common-python/latest/wrapping-guide/) +documents the conventions for writing wrappers: package layout, type design, +conversion functions, and the field-name and docstring rules. ## Releasing diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 29f32f5d..44b3c0c0 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -175,6 +175,12 @@ * 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 three authored documentation guides, one per audience: + + * **User Guide** — For users of the wrapper types: typed IDs, safe accessors and exceptions, numeric types, enum-or-int fields, validity in the type, membership and bounds, reading string output, and an overview of the available wrappers. + * **Client Developer Guide** — For `frequenz-client-*` library authors: which versioned `proto.v1alphaN` package to import, the usual client-method shapes, what this library already converts, and when to write your own wrappers. + * **Wrapping Guide** — For anyone designing a wrapper: package layout, enum representation, data types, validity in the type, conversion functions, deprecation, and testing. The field-name and docstring rules previously listed in `CONTRIBUTING.md` moved here. + ## 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. diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index 3755def6..94206fbe 100644 --- a/docs/SUMMARY.md +++ b/docs/SUMMARY.md @@ -1,3 +1,6 @@ * [Home](index.md) +* [User Guide](user-guide/) +* [Client Developer Guide](client-developer-guide/) +* [Wrapping Guide](wrapping-guide/) * [API Reference](reference/) * [Contributing](CONTRIBUTING.md) diff --git a/docs/client-developer-guide/SUMMARY.md b/docs/client-developer-guide/SUMMARY.md new file mode 100644 index 00000000..adf4b45e --- /dev/null +++ b/docs/client-developer-guide/SUMMARY.md @@ -0,0 +1,5 @@ +* [Overview](index.md) +* [Namespace and versioning](namespace-and-versioning.md) +* [Using conversion functions](using-conversion-functions.md) +* [Conversion functions provided by this library](shipped-converters.md) +* [Building your own wrappers](building-your-own.md) diff --git a/docs/client-developer-guide/building-your-own.md b/docs/client-developer-guide/building-your-own.md new file mode 100644 index 00000000..435aeb0f --- /dev/null +++ b/docs/client-developer-guide/building-your-own.md @@ -0,0 +1,41 @@ +# Building your own wrappers + +This library provides high-level Python wrappers and conversion functions for +messages from +[`frequenz-api-common`](https://github.com/frequenz-floss/frequenz-api-common). +If your client receives protobuf messages from a service-specific API, write +the wrapper and its conversion functions in your own client library. + +## Choose where to write the wrapper + +Use the conversion functions this library provides for messages from +`frequenz-api-common`. They let every client return the same high-level Python +wrapper types. + +Write a custom wrapper in your own client library for messages from a +service-specific API. Keep low-level protobuf types inside your conversion +functions. Your public methods should only expose high-level Python wrappers. + +## Follow the Wrapping Guide + +When you write custom wrappers and conversion functions, use the patterns in the +[Wrapping Guide](../wrapping-guide/index.md): + +- [Organizing a wrapper package](../wrapping-guide/organizing-a-wrapper-package.md) + shows how to keep low-level protobuf imports out of wrapper modules. +- [Enums](../wrapping-guide/enums.md) and + [Data types](../wrapping-guide/data-types.md) show Python types for protobuf + fields and enums. +- [Validity in the type](../wrapping-guide/validity-in-the-type.md) and + [Conversion functions](../wrapping-guide/conversion-functions.md) show how to + represent invalid values, unknown enum numbers, and optional or `oneof` fields. +- [Deprecation and compatibility](../wrapping-guide/deprecation-and-compatibility.md) + and [Testing](../wrapping-guide/testing.md) show how to change wrapper APIs + safely and test conversion functions. + +## Use shared wrappers in your custom conversion functions + +When a service-specific protobuf message has a nested `frequenz-api-common` +message, call one of this library's conversion functions for that field. It +returns the shared high-level Python wrapper without making you write the same +translation again. diff --git a/docs/client-developer-guide/index.md b/docs/client-developer-guide/index.md new file mode 100644 index 00000000..9e844307 --- /dev/null +++ b/docs/client-developer-guide/index.md @@ -0,0 +1,28 @@ +# Client Developer Guide + +This guide is for developers building `frequenz-client-*` libraries with this +library. Your client gets low-level protobuf messages from a gRPC service. The +conversion functions this library provides turn them into high-level Python +wrappers that you return to your users. + +If this library does not provide a wrapper you need, see the +[Wrapping Guide](../wrapping-guide/index.md). The [User Guide](../user-guide/index.md) +explains how users can work with the wrappers your client returns. + +## Sections + +- [Namespace and versioning](namespace-and-versioning.md) — Shows how the + conversion functions are grouped by protobuf API version. Import the group + that matches the messages your service sends. + +- [Using conversion functions](using-conversion-functions.md) — Shows the two + usual client-method shapes: one protobuf message and a list of them. Your + methods convert the messages and return the wrappers. + +- [Conversion functions provided by this library](shipped-converters.md) — Lists + the `v1alpha8` packages and the messages they translate. The table links each + package to its API reference. + +- [Building your own wrappers](building-your-own.md) — Explains when your client + library needs its own wrapper types and conversion functions. It points to the + Wrapping Guide for the patterns to use. diff --git a/docs/client-developer-guide/namespace-and-versioning.md b/docs/client-developer-guide/namespace-and-versioning.md new file mode 100644 index 00000000..e4766347 --- /dev/null +++ b/docs/client-developer-guide/namespace-and-versioning.md @@ -0,0 +1,52 @@ +# Namespace and versioning + +The conversion functions this library provides are grouped by the version of +the [`frequenz-api-common`](https://github.com/frequenz-floss/frequenz-api-common) +protobuf API they understand. Right now, this library provides `v1alpha8`. +Import the group that matches the messages your client receives. + +## Import the matching version + +When your client receives messages from the `v1alpha8` protobuf API, import +conversion functions from `frequenz.client.common..proto.v1alpha8`. + +For example, +[`location_from_proto`][frequenz.client.common.types.proto.v1alpha8.location_from_proto] +translates a protobuf +[`location_pb2.Location`][frequenz.api.common.v1alpha8.types.location_pb2.Location] +message into a high-level +[`Location`][frequenz.client.common.types.Location] wrapper: + +```python +from frequenz.client.common.types.proto.v1alpha8 import location_from_proto + +location = location_from_proto(response.location) +``` + +Your client works with the returned +[`Location`][frequenz.client.common.types.Location] wrapper. + +## Keep your import and messages on the same version + +The version of the protobuf messages determines which package you import. The +release version of this library does not. + +- Import `frequenz.client.common..proto.v1alpha8` for messages from the + `v1alpha8` protobuf API. +- Use `v1alphaN` only after your client uses that protobuf API version. + +## When a new protobuf API version is available + +When this library adds another protobuf API version, it adds a sibling package: + +```text +types/ +└── proto/ + ├── v1alpha8/ + │ └── __init__.py + └── v1alphaN/ + └── __init__.py +``` + +Clients that use `v1alpha8` keep importing `v1alpha8`. When your client moves +to a newer protobuf API version, update its imports to the matching package. diff --git a/docs/client-developer-guide/shipped-converters.md b/docs/client-developer-guide/shipped-converters.md new file mode 100644 index 00000000..f81ec20c --- /dev/null +++ b/docs/client-developer-guide/shipped-converters.md @@ -0,0 +1,38 @@ +# Conversion functions provided by this library + +The conversion functions this library provides translate the low-level protobuf +messages from +[`frequenz-api-common`](https://github.com/frequenz-floss/frequenz-api-common) +into high-level Python wrappers. This page lists the packages that contain them +and links to their API reference pages. + +## v1alpha8 conversion packages + +Right now, this library provides packages for the `v1alpha8` protobuf API. +Import the package that matches the messages your code receives. + +| Package | What it translates | +| --- | --- | +| [`grid`][frequenz.client.common.grid.proto.v1alpha8] | Delivery areas ([`delivery_area_pb2.DeliveryArea`][frequenz.api.common.v1alpha8.grid.delivery_area_pb2.DeliveryArea]) through [`delivery_area_from_proto2`][frequenz.client.common.grid.proto.v1alpha8.delivery_area_from_proto2], plus energy-market code-type enum conversion. | +| [`metrics`][frequenz.client.common.metrics.proto.v1alpha8] | Metric samples ([`metrics_pb2.MetricSample`][frequenz.api.common.v1alpha8.metrics.metrics_pb2.MetricSample]), connections, aggregate values, bounds and bounds sets, plus metric and connection-category enum conversion. | +| [`microgrid`][frequenz.client.common.microgrid.proto.v1alpha8] | Microgrids ([`microgrid_pb2.Microgrid`][frequenz.api.common.v1alpha8.microgrid.microgrid_pb2.Microgrid]) and lifetimes through [`microgrid_from_proto`][frequenz.client.common.microgrid.proto.v1alpha8.microgrid_from_proto] and [`lifetime_from_proto`][frequenz.client.common.microgrid.proto.v1alpha8.lifetime_from_proto]. | +| [`microgrid.electrical_components`][frequenz.client.common.microgrid.electrical_components.proto.v1alpha8] | Electrical-component classes ([`electrical_components_pb2.ElectricalComponent`][frequenz.api.common.v1alpha8.microgrid.electrical_components.electrical_components_pb2.ElectricalComponent]), component instances and connections, plus category, diagnostic-code, and state-code enums. | +| [`pagination`][frequenz.client.common.pagination.proto.v1alpha8] | Pagination information ([`pagination_info_pb2.PaginationInfo`][frequenz.api.common.v1alpha8.pagination.pagination_info_pb2.PaginationInfo]) in both directions through [`pagination_info_from_proto`][frequenz.client.common.pagination.proto.v1alpha8.pagination_info_from_proto] and [`pagination_info_to_proto`][frequenz.client.common.pagination.proto.v1alpha8.pagination_info_to_proto]. | +| [`streaming`][frequenz.client.common.streaming.proto.v1alpha8] | Streaming events ([`event_pb2.Event`][frequenz.api.common.v1alpha8.streaming.event_pb2.Event]) in both directions through [`event_from_proto`][frequenz.client.common.streaming.proto.v1alpha8.event_from_proto] and [`event_to_proto`][frequenz.client.common.streaming.proto.v1alpha8.event_to_proto]. | +| [`types`][frequenz.client.common.types.proto.v1alpha8] | Locations ([`location_pb2.Location`][frequenz.api.common.v1alpha8.types.location_pb2.Location]) through [`location_from_proto`][frequenz.client.common.types.proto.v1alpha8.location_from_proto]. | + +## Choose a conversion direction + +Call a `*_from_proto` function after your code receives a low-level protobuf +message over gRPC. It returns a high-level Python wrapper. Call a `*_to_proto` +function when your code builds a low-level protobuf message to send over gRPC. +Some packages only provide one direction because their protobuf API only needs +one direction. + +Import from the public package in the table, such as +`frequenz.client.common.types.proto.v1alpha8`. Do not import from an internal +module whose name starts with an underscore. + +Use the API reference links in the table for parameters, return types, and the +full function lists. For the usual client-method pattern, see +[Using conversion functions](using-conversion-functions.md). diff --git a/docs/client-developer-guide/using-conversion-functions.md b/docs/client-developer-guide/using-conversion-functions.md new file mode 100644 index 00000000..ba1b9e83 --- /dev/null +++ b/docs/client-developer-guide/using-conversion-functions.md @@ -0,0 +1,27 @@ +# Using conversion functions + +Your client library calls a gRPC service and gets back low-level protobuf +messages. The conversion functions this library provides translate those into +the high-level Python wrappers, so the rest of your code — and your users — +work with wrappers instead of protobuf. + +## Convert a single message + +```python +response = await self._stub.GetMetricSample(request) +return metric_sample_from_proto(response.metric_sample) +``` + +## Convert a list of messages + +```python +response = await self._stub.ListMetricSamples(request) +return [metric_sample_from_proto(s) for s in response.samples] +``` + +## You don't handle unknown or invalid values here + +The conversion function already deals with them: it keeps an enum value your +version doesn't recognize as a plain `int`, and returns an `Invalid*` wrapper +instead of raising when data is malformed. You just return the wrapper. +Deciding what those cases *mean* is up to your users — see the [User Guide](../user-guide/index.md). diff --git a/docs/user-guide/SUMMARY.md b/docs/user-guide/SUMMARY.md new file mode 100644 index 00000000..18edc705 --- /dev/null +++ b/docs/user-guide/SUMMARY.md @@ -0,0 +1,9 @@ +* [Overview](index.md) +* [Typed IDs](typed-ids.md) +* [Safe accessors & exceptions](safe-accessors.md) +* [Numeric types](numeric-types.md) +* [Enum-or-int fields](enum-or-int-fields.md) +* [Validity in the type](validity-in-the-type.md) +* [Membership & bounds](membership-and-bounds.md) +* [Reading string output](reading-string-output.md) +* [Overview of available wrappers](overview.md) diff --git a/docs/user-guide/enum-or-int-fields.md b/docs/user-guide/enum-or-int-fields.md new file mode 100644 index 00000000..04326337 --- /dev/null +++ b/docs/user-guide/enum-or-int-fields.md @@ -0,0 +1,136 @@ +# Enum-or-int fields + +Some fields are an enum member or an integer, such as +[`MetricSample.metric`][frequenz.client.common.metrics.MetricSample.metric]. +The integer keeps newer server values available to an older client instead of +failing when the client does not recognize them. + +Prefer a safe accessor when one is available. For example, +[`MetricSample.get_metric()`][frequenz.client.common.metrics.MetricSample.get_metric] +returns a known [`Metric`][frequenz.client.common.metrics.Metric] member. It +raises [`UnspecifiedEnumValueError`][frequenz.client.common.UnspecifiedEnumValueError] for the raw value `0` and +[`UnrecognizedEnumValueError`][frequenz.client.common.UnrecognizedEnumValueError] for another unrecognized integer. See [safe +accessors](safe-accessors.md) to handle those exceptions. + +```python +from datetime import datetime, timezone + +from frequenz.client.common import ( + UnrecognizedEnumValueError, + UnspecifiedEnumValueError, +) +from frequenz.client.common.metrics import BoundsSet, Metric, MetricSample + + +def sample_with(metric: Metric | int) -> MetricSample: + return MetricSample( + sample_time=datetime(2026, 1, 1, tzinfo=timezone.utc), + metric=metric, + value=42.0, + bounds_set=BoundsSet(), + ) + + +for sample in ( + sample_with(Metric.AC_POWER_ACTIVE), + sample_with(0), + sample_with(999), +): + try: + print(sample.get_metric().name) + except UnspecifiedEnumValueError: + print("unspecified") + except UnrecognizedEnumValueError as error: + print(f"unrecognized: {error.value}") +``` + +When you need the lower-level field, dispatch with `match`. A known member +matches the enum, raw `0` means unspecified, and any other integer is an +unrecognized value. Do not use `isinstance()` or look for an `UNSPECIFIED` +member. + +You only read these values. Treat the raw integer `0` as unspecified rather +than setting an enum's `UNSPECIFIED` member. + +```python +from datetime import datetime, timezone +from typing import assert_never + +from frequenz.client.common.metrics import BoundsSet, Metric, MetricSample + + +def describe(sample: MetricSample) -> str: + match sample.metric: + case Metric() as metric: + return f"known: {metric.name}" + case 0: + return "unspecified" + case int() as value: + return f"unrecognized: {value}" + case unexpected: + assert_never(unexpected) + + +samples = ( + MetricSample( + sample_time=datetime(2026, 1, 1, tzinfo=timezone.utc), + metric=Metric.AC_POWER_ACTIVE, + value=42.0, + bounds_set=BoundsSet(), + ), + MetricSample( + sample_time=datetime(2026, 1, 1, tzinfo=timezone.utc), + metric=0, + value=42.0, + bounds_set=BoundsSet(), + ), + MetricSample( + sample_time=datetime(2026, 1, 1, tzinfo=timezone.utc), + metric=999, + value=42.0, + bounds_set=BoundsSet(), + ), +) + +for sample in samples: + print(describe(sample)) +``` + +An unrecognized integer is not invalid data. It is a value the server knows +that this client version does not yet recognize. Invalid values are a separate +topic; see [Validity in the type](validity-in-the-type.md). + +## Treat the numbers as opaque + +Work with members, not numbers. Compare against the member itself, as in +`sample.metric is Metric.AC_POWER_ACTIVE`, and let the accessors and `match` +arms above do the rest. A number is how the protocol identifies a value. It is +not how your code should identify it. + +Reach for `.value` only as a last resort, when you must hand a raw protocol +number to something that speaks the protocol itself. The mapping is direct: a +member name is the protocol name without its fixed prefix, and a member value is +the protocol number. + +```python +from frequenz.client.common.metrics import Metric + +# A last resort: Metric.AC_POWER_ACTIVE is METRIC_AC_POWER_ACTIVE in the protocol. +assert Metric.AC_POWER_ACTIVE.value == 26 +``` + +Those numbers belong to a protocol version, and only that version gives them a +meaning. The same number can name different things in two versions. When a new +protocol version gives an existing name a different number, this library follows +the newest version it supports. A member's number therefore changes only in a +release that adds support for a new protocol version. That is a +[breaking change](https://github.com/frequenz-floss/docs/blob/v0.x.x/python/semver-0.x.x.md) +and the release notes call it out, but the member name stays the same. + +A raw integer needs the same care, and more. Read it as "the server sent a value +this client does not know", not as a stable identifier. Do not persist it or +pass it to another system as if it were version-independent, and do not compare +it against a member number to guess which value it is. To learn what one means, +look it up in the [`frequenz-api-common`](https://github.com/frequenz-floss/frequenz-api-common) +definition for the version your client speaks, or upgrade this library and your +client so the value resolves to a member. diff --git a/docs/user-guide/index.md b/docs/user-guide/index.md new file mode 100644 index 00000000..1cf7949b --- /dev/null +++ b/docs/user-guide/index.md @@ -0,0 +1,42 @@ +# User Guide + +`frequenz-client-common` wraps the raw protobuf messages of the Frequenz common +API in idiomatic, type-safe Python. Frequenz API client libraries build on it +and hand you these wrapper objects — typed IDs, metrics, bounds, locations, and +more — so you work with natural Python types instead of generated protobuf code. + +This guide shows you how to use those objects safely: reading their values, +handling data that may be missing or invalid, and relying on the types to catch +mistakes early. You don't need to know anything about protobuf to follow it. +If you are building a client library instead, see the +[Client Developer Guide](../client-developer-guide/index.md). + +## Sections + +- [Typed IDs](typed-ids.md) — Shows how typed identifiers distinguish + microgrids, components, sensors, and enterprises even when they have the + same number. It also shows how to print, compare, and use them as keys. +- [Safe accessors & exceptions](safe-accessors.md) — Explains when `get_*()` + accessors return a validated value and when they raise an exception. Use it + when a field may be missing, invalid, or unrecognized. +- [Numeric types](numeric-types.md) — Explains why a numeric value may be a + `float` or an `int` at runtime and how to handle either safely. It also + covers the special case of boolean values. +- [Enum-or-int fields](enum-or-int-fields.md) — Shows why some fields can + contain an enum member or a raw integer. Learn how to handle unspecified and + unrecognized values with accessors or `match`, and why you should treat the + underlying numbers as opaque values tied to a protocol version. +- [Validity in the type](validity-in-the-type.md) — Shows how wrappers retain + invalid values instead of dropping them. Learn how to inspect those values + or let a safe accessor raise an exception. +- [Membership & bounds](membership-and-bounds.md) — Shows how + [`Bounds`][frequenz.client.common.metrics.Bounds] and + [`BoundsSet`][frequenz.client.common.metrics.BoundsSet] support `in` range + checks. It explains bounded and unbounded ranges and what to do with invalid + bounds. +- [Reading string output](reading-string-output.md) — Explains markers such as + `` and unexpected raw values in logs. Learn which indicate invalid + data and which indicate data that this client does not recognize yet. +- [Overview of available wrappers](overview.md) — Lists wrapper types by data + group and links to their API reference. Use it to find the types for the + common data you receive. diff --git a/docs/user-guide/membership-and-bounds.md b/docs/user-guide/membership-and-bounds.md new file mode 100644 index 00000000..6d368a97 --- /dev/null +++ b/docs/user-guide/membership-and-bounds.md @@ -0,0 +1,88 @@ +# Membership & bounds + +Use `in` to check whether a value is within a +[`Bounds`][frequenz.client.common.metrics.Bounds] range. Endpoints are +inclusive, so the values at either end are members too. + +Create [`Bounds`][frequenz.client.common.metrics.Bounds] with optional lower and +upper endpoints. `-inf` for a lower endpoint and `+inf` for an upper endpoint +become `None`; reversed endpoints and `NaN` raise [`ValueError`][]. + +```python +from frequenz.client.common.metrics import Bounds, BoundsSet + +bounds = Bounds(lower=0, upper=100) +bounds_set = BoundsSet([bounds]) +unbounded = Bounds() + +print(50 in bounds) # True +print(150 in bounds) # False +print(0 in bounds) # True +print(100 in bounds) # True +print(50 in bounds_set) # True +print(bool(unbounded)) # False +``` + +A `None` endpoint leaves that direction unbounded. A lower endpoint of `0` +accepts any value at least `0`, while bounds without endpoints accept every +numeric value. `None` and `NaN` are never members. You can use +[`Bounds.is_bounded()`][frequenz.client.common.metrics.Bounds.is_bounded] or +truthiness when you need to distinguish a restricted range from a fully +unbounded one. + +Test a [`BoundsSet`][frequenz.client.common.metrics.BoundsSet] in the same way. +It stores a normalized union: overlapping or touching ranges are merged, and a +union that covers the full numeric range is stored as an empty +[`BoundsSet.bounds`][frequenz.client.common.metrics.BoundsSet.bounds] tuple. That +empty tuple represents the unbounded set and still contains every numeric value +except `NaN`. Direct membership with `value in bounds_set` is authoritative; do +not reconstruct it by iterating over `bounds_set.bounds`. + +```python +from frequenz.client.common.metrics import Bounds, BoundsSet + +unbounded_set = BoundsSet([Bounds()]) + +assert unbounded_set.bounds == () +assert 42 in unbounded_set +``` + +[`BoundsSet`][frequenz.client.common.metrics.BoundsSet] accepts any iterable of +[`Bounds`][frequenz.client.common.metrics.Bounds], such as a list, passed +positionally—not only a tuple. + +When you read a metric sample, prefer +[`MetricSample.get_bounds_set()`][frequenz.client.common.metrics.MetricSample.get_bounds_set]. +It returns a valid [`BoundsSet`][frequenz.client.common.metrics.BoundsSet] or +raises +[`InvalidBoundsSetError`][frequenz.client.common.metrics.InvalidBoundsSetError]. +See [safe accessors](safe-accessors.md) for handling the exception. + +If you need the lower-level +[`MetricSample.bounds_set`][frequenz.client.common.metrics.MetricSample.bounds_set] +field, handle both cases explicitly: + +```python +from typing import assert_never + +from frequenz.client.common.metrics import BoundsSet, InvalidBoundsSet + + +def describe(bounds_set: BoundsSet | InvalidBoundsSet) -> str: + match bounds_set: + case BoundsSet(): + return "valid" + case InvalidBoundsSet(): + return "malformed" + case unexpected: + assert_never(unexpected) + + +print(describe(BoundsSet())) # valid +``` + +[`InvalidBounds`][frequenz.client.common.metrics.InvalidBounds] and +[`InvalidBoundsSet`][frequenz.client.common.metrics.InvalidBoundsSet] retain +malformed data for inspection. Do not use either for range checks; use the +safe accessor or handle the union as above. See [validity in the +type](validity-in-the-type.md) for this pattern. diff --git a/docs/user-guide/numeric-types.md b/docs/user-guide/numeric-types.md new file mode 100644 index 00000000..9e090d6d --- /dev/null +++ b/docs/user-guide/numeric-types.md @@ -0,0 +1,43 @@ +# Numeric types + +Numeric values can be an integer or a floating-point number. Handle them as +numbers without assuming a particular runtime type. + +```python +from typing import assert_never + +from frequenz.core.typing import FloatInt + + +def describe(value: FloatInt | None) -> str: + match value: + case float() | int(): + return f"next value: {value + 1}" + case None: + return "no value" + case unexpected: + assert_never(unexpected) + + +print(describe(1.5)) # next value: 2.5 +print(describe(2)) # next value: 3 +print(describe(None)) # no value +``` + +[`FloatInt`][frequenz.core.typing.FloatInt] is exactly `float | int`. +[PEP 484's numeric tower](https://peps.python.org/pep-0484/#the-numeric-tower) +allows an `int` where a `float` is annotated, so a [`FloatInt`][frequenz.core.typing.FloatInt] value may be a +real `float` or `int` at runtime. Arithmetic and comparisons work the same for +both. Do not use `isinstance(value, float)` alone: it is `False` for an `int`. + +You can encounter [`FloatInt`][frequenz.core.typing.FloatInt] in +[`MetricSample.value`][frequenz.client.common.metrics.MetricSample.value] and +from +[`MetricSample.as_single_value()`][frequenz.client.common.metrics.MetricSample.as_single_value]. +[`Bounds`][frequenz.client.common.metrics.Bounds] also uses it for `lower` and +`upper`. When you need to dispatch on the concrete type, use the `match` form +above so both number types are handled together. + +`bool` is a subclass of `int`, so `True` and `False` also satisfy [`FloatInt`][frequenz.core.typing.FloatInt]. +Handle them separately only when your application does not accept boolean +values. diff --git a/docs/user-guide/overview.md b/docs/user-guide/overview.md new file mode 100644 index 00000000..270dc565 --- /dev/null +++ b/docs/user-guide/overview.md @@ -0,0 +1,92 @@ +# Overview of available wrappers + +The library groups its wrappers by the kind of common API data you receive. +Use this map to find the relevant domain, then follow its links for the public +API details. + +## Grid + +Start with [`DeliveryArea`][frequenz.client.common.grid.DeliveryArea] and +[`EnergyMarketCodeType`][frequenz.client.common.grid.EnergyMarketCodeType]. +[`InvalidDeliveryArea`][frequenz.client.common.grid.InvalidDeliveryArea], +[`BaseDeliveryArea`][frequenz.client.common.grid.BaseDeliveryArea], and +[`InvalidDeliveryAreaError`][frequenz.client.common.grid.InvalidDeliveryAreaError] +cover invalid delivery-area data. + +## Metrics + +The main types are +[`Metric`][frequenz.client.common.metrics.Metric], +[`MetricSample`][frequenz.client.common.metrics.MetricSample], +[`MetricConnection`][frequenz.client.common.metrics.MetricConnection], +[`MetricConnectionCategory`][frequenz.client.common.metrics.MetricConnectionCategory], +[`Bounds`][frequenz.client.common.metrics.Bounds], +[`BoundsSet`][frequenz.client.common.metrics.BoundsSet], +[`AggregatedMetricValue`][frequenz.client.common.metrics.AggregatedMetricValue], and +[`AggregationMethod`][frequenz.client.common.metrics.AggregationMethod]. The +[`BaseBounds`][frequenz.client.common.metrics.BaseBounds], +[`InvalidBounds`][frequenz.client.common.metrics.InvalidBounds], +[`InvalidBoundsError`][frequenz.client.common.metrics.InvalidBoundsError], +[`InvalidBoundsSet`][frequenz.client.common.metrics.InvalidBoundsSet], and +[`InvalidBoundsSetError`][frequenz.client.common.metrics.InvalidBoundsSetError] +cover the base and invalid range variants. + +## Microgrid + +Use [`Microgrid`][frequenz.client.common.microgrid.Microgrid], +[`MicrogridId`][frequenz.client.common.microgrid.MicrogridId], +[`EnterpriseId`][frequenz.client.common.microgrid.EnterpriseId], and +[`Lifetime`][frequenz.client.common.microgrid.Lifetime]. +[`BaseLifetime`][frequenz.client.common.microgrid.BaseLifetime] and +[`InvalidLifetime`][frequenz.client.common.microgrid.InvalidLifetime] represent +the lifetime variants, while +[`InvalidLifetimeError`][frequenz.client.common.microgrid.InvalidLifetimeError] +is raised by safe accessors. + +## Electrical components + +The common base is +[`ElectricalComponent`][frequenz.client.common.microgrid.electrical_components.ElectricalComponent], +with [`ElectricalComponentId`][frequenz.client.common.microgrid.electrical_components.ElectricalComponentId] +for component identities. Concrete categories include +[`Battery`][frequenz.client.common.microgrid.electrical_components.Battery] and +[`LiIonBattery`][frequenz.client.common.microgrid.electrical_components.LiIonBattery]/[`NaIonBattery`][frequenz.client.common.microgrid.electrical_components.NaIonBattery], +the [`Inverter`][frequenz.client.common.microgrid.electrical_components.Inverter] +family, the [`EvCharger`][frequenz.client.common.microgrid.electrical_components.EvCharger] +family, [`Meter`][frequenz.client.common.microgrid.electrical_components.Meter], +and [`GridConnectionPoint`][frequenz.client.common.microgrid.electrical_components.GridConnectionPoint]. + +Recovery types preserve data this version cannot classify, including +[`UnspecifiedElectricalComponent`][frequenz.client.common.microgrid.electrical_components.UnspecifiedElectricalComponent], +[`UnrecognizedElectricalComponent`][frequenz.client.common.microgrid.electrical_components.UnrecognizedElectricalComponent], +[`UnspecifiedBattery`][frequenz.client.common.microgrid.electrical_components.UnspecifiedBattery], +[`UnrecognizedBattery`][frequenz.client.common.microgrid.electrical_components.UnrecognizedBattery], +and [`MismatchedCategoryElectricalComponent`][frequenz.client.common.microgrid.electrical_components.MismatchedCategoryElectricalComponent]. +See the [API Reference](../reference/frequenz/client/common/index.md) for the complete component list. + +## Sensors + +This namespace provides [`SensorId`][frequenz.client.common.microgrid.sensors.SensorId] +for sensor identities. + +## Common types + +Use [`Location`][frequenz.client.common.types.Location] for location data; +[`InvalidLatitude`][frequenz.client.common.types.InvalidLatitude], +[`InvalidLongitude`][frequenz.client.common.types.InvalidLongitude], and +[`InvalidCountryCode`][frequenz.client.common.types.InvalidCountryCode] preserve +invalid fields. + +## Streaming + +[`Event`][frequenz.client.common.streaming.Event] is the event wrapper. + +## Pagination + +[`PaginationInfo`][frequenz.client.common.pagination.PaginationInfo] carries +pagination details. + +For every field, method, and remaining wrapper type, see the [API +Reference](../reference/frequenz/client/common/index.md). The +[Client Developer Guide](../client-developer-guide/index.md) explains how +client libraries return these wrappers. diff --git a/docs/user-guide/reading-string-output.md b/docs/user-guide/reading-string-output.md new file mode 100644 index 00000000..ff674825 --- /dev/null +++ b/docs/user-guide/reading-string-output.md @@ -0,0 +1,36 @@ +# Reading string output + +Wrapper objects have compact string representations for logs and debugging. +Read an `` marker as an invariant violation: the object preserves +malformed data, but that field is not valid. + +```python +from frequenz.client.common.metrics import InvalidBounds +from frequenz.client.common.types import InvalidLatitude + +latitude = InvalidLatitude(value=999.0) +bounds = InvalidBounds(lower=10, upper=5) + +print(latitude) # +print(bounds) # +``` + +[`InvalidLatitude`][frequenz.client.common.types.InvalidLatitude] keeps an +out-of-range latitude, while +[`InvalidBounds`][frequenz.client.common.metrics.InvalidBounds] keeps a +malformed pair of bounds. [`InvalidLongitude`][frequenz.client.common.types.InvalidLongitude] +and [`InvalidCountryCode`][frequenz.client.common.types.InvalidCountryCode] +use the same marker. See [validity in the type](validity-in-the-type.md) to +handle these values. + +An unexpected raw number without `` means something different: the +data is well-formed, but this client version does not recognize it yet. For +example, [`UnrecognizedElectricalComponent`][frequenz.client.common.microgrid.electrical_components.UnrecognizedElectricalComponent] +prints a suffix shaped like `:category=`, and +[`UnrecognizedBattery`][frequenz.client.common.microgrid.electrical_components.UnrecognizedBattery] +prints `:type=`. Those plain values are forward-compatible unknown data, +not invalid data. See [enum-or-int fields](enum-or-int-fields.md) for handling +that case. + +Use these strings for human inspection and logging, not programmatic parsing. +Use the typed fields and accessors for application logic instead. diff --git a/docs/user-guide/safe-accessors.md b/docs/user-guide/safe-accessors.md new file mode 100644 index 00000000..d53cb129 --- /dev/null +++ b/docs/user-guide/safe-accessors.md @@ -0,0 +1,65 @@ +# Safe accessors & exceptions + +Some fields can contain a value the library cannot use safely. Prefer a +`get_*()` accessor when you need that value: it returns a validated value or +raises a specific exception. + +```python +from datetime import datetime, timezone + +from frequenz.client.common.metrics import BoundsSet, Metric, MetricSample + +sample = MetricSample( + sample_time=datetime(2026, 1, 1, tzinfo=timezone.utc), + metric=Metric.AC_POWER_ACTIVE, + value=42.0, + bounds_set=BoundsSet(), +) + +metric: Metric = sample.get_metric() +print(metric.name) # AC_POWER_ACTIVE +``` + +[`MetricSample.get_metric()`][frequenz.client.common.metrics.MetricSample.get_metric] +returns a known [`Metric`][frequenz.client.common.metrics.Metric]. Reading +[`MetricSample.metric`][frequenz.client.common.metrics.MetricSample.metric] +directly can instead give you a lower-level integer that needs checking. + +```python +from datetime import datetime, timezone + +from frequenz.client.common import UnrecognizedEnumValueError +from frequenz.client.common.metrics import BoundsSet, MetricSample + +sample = MetricSample( + sample_time=datetime(2026, 1, 1, tzinfo=timezone.utc), + metric=999, + value=42.0, + bounds_set=BoundsSet(), +) + +try: + sample.get_metric() +except UnrecognizedEnumValueError as error: + print(error.attr_name) # metric + print(error.value) # 999 +``` + +Catch [`UnrecognizedEnumValueError`][frequenz.client.common.UnrecognizedEnumValueError] +when you need to handle an unrecognized value and inspect its raw integer. +[`UnspecifiedEnumValueError`][frequenz.client.common.UnspecifiedEnumValueError] +handles the unspecified value. Other accessors follow the same pattern; for +example, +[`MetricSample.get_bounds_set()`][frequenz.client.common.metrics.MetricSample.get_bounds_set] +returns a valid [`BoundsSet`][frequenz.client.common.metrics.BoundsSet] or +raises [`InvalidBoundsSetError`][frequenz.client.common.metrics.InvalidBoundsSetError]. + +For a shared fallback, catch +[`InvalidAttributeError`][frequenz.client.common.InvalidAttributeError] for any +invalid field value, including a +[`MissingFieldError`][frequenz.client.common.MissingFieldError]. It is also a +[`ValueError`][]. Catch +[`ClientCommonError`][frequenz.client.common.ClientCommonError] when you need +to handle any library-defined semantic accessor error. Constructors, +conversion functions, and normal Python operations can also raise built-in +exceptions such as `ValueError` or `TypeError`. diff --git a/docs/user-guide/typed-ids.md b/docs/user-guide/typed-ids.md new file mode 100644 index 00000000..83e55aa3 --- /dev/null +++ b/docs/user-guide/typed-ids.md @@ -0,0 +1,37 @@ +# Typed IDs + +Use typed IDs when you work with microgrids and their resources. Instead of +plain integers, the library gives you distinct types such as +[`MicrogridId`][frequenz.client.common.microgrid.MicrogridId], +[`ElectricalComponentId`][frequenz.client.common.microgrid.electrical_components.ElectricalComponentId], +[`SensorId`][frequenz.client.common.microgrid.sensors.SensorId], and +[`EnterpriseId`][frequenz.client.common.microgrid.EnterpriseId]. This makes an +ID self-describing when you print it and lets a type checker catch using a +sensor ID where a microgrid ID is expected. + +```python +from frequenz.client.common.microgrid import MicrogridId +from frequenz.client.common.microgrid.sensors import SensorId + +microgrid_id = MicrogridId(123) +same_microgrid_id = MicrogridId(123) +sensor_id = SensorId(123) + +print(microgrid_id) # MID123 +print(microgrid_id == same_microgrid_id) # True +print(microgrid_id == sensor_id) # False + +ids = {microgrid_id, same_microgrid_id, sensor_id} +print(len(ids)) # 2 + +names = {microgrid_id: "Rooftop microgrid"} +print(names[MicrogridId(123)]) # Rooftop microgrid +``` + +Construct an ID from its numeric value. IDs of the same type and value compare +equal, so you can use them as dictionary keys or set members. IDs with the +same number but different types are not equal. + +The printed prefix identifies the kind of ID: `MID` for a microgrid, `CID` for +an electrical component, `SID` for a sensor, and `EID` for an enterprise. All +of these types are based on [`BaseId`][frequenz.core.id.BaseId]. diff --git a/docs/user-guide/validity-in-the-type.md b/docs/user-guide/validity-in-the-type.md new file mode 100644 index 00000000..d037987f --- /dev/null +++ b/docs/user-guide/validity-in-the-type.md @@ -0,0 +1,112 @@ +# Validity in the type + +When data violates an invariant, the library keeps the raw value instead of +silently dropping it or failing immediately. The type makes the invalid value +visible, so you can decide how to handle it. + +Some fields return a whole object or its invalid counterpart. For example, a +delivery area can be a [`DeliveryArea`][frequenz.client.common.grid.DeliveryArea] +or an [`InvalidDeliveryArea`][frequenz.client.common.grid.InvalidDeliveryArea]. +Match both cases when you read such a value: + +```python +from typing import assert_never + +from frequenz.client.common.grid import DeliveryArea, InvalidDeliveryArea + + +def describe(area: DeliveryArea | InvalidDeliveryArea) -> str: + match area: + case DeliveryArea(): + return "valid" + case InvalidDeliveryArea(): + return "invalid" + case unexpected: + assert_never(unexpected) +``` + +An invalid object keeps its raw fields. Prefer a safe accessor when the object +offers one: it returns the valid value or raises a typed +[`InvalidAttributeError`][frequenz.client.common.InvalidAttributeError] subclass. +For example, an [`InvalidDeliveryAreaError`][frequenz.client.common.grid.InvalidDeliveryAreaError] exposes the invalid value on +[`InvalidDeliveryAreaError.delivery_area`][frequenz.client.common.grid.InvalidDeliveryAreaError.delivery_area]. See [safe accessors](safe-accessors.md). + +An otherwise valid object can also carry an invalid value in one field. +[`Location`][frequenz.client.common.types.Location] uses +[`InvalidLatitude`][frequenz.client.common.types.InvalidLatitude], +[`InvalidLongitude`][frequenz.client.common.types.InvalidLongitude], and +[`InvalidCountryCode`][frequenz.client.common.types.InvalidCountryCode] wrappers. +Each has the raw value on [`InvalidLatitude.value`][frequenz.client.common.types.InvalidLatitude.value], +[`InvalidLongitude.value`][frequenz.client.common.types.InvalidLongitude.value], or +[`InvalidCountryCode.value`][frequenz.client.common.types.InvalidCountryCode.value]. + +```python +from frequenz.client.common.types import ( + InvalidCountryCode, + InvalidLatitude, + InvalidLongitude, + Location, +) + +location = Location( + latitude=InvalidLatitude(value=999.0), + longitude=InvalidLongitude(value=-999.0), + country_code=InvalidCountryCode(value="Germany"), +) + +match location.latitude: + case InvalidLatitude(value=raw_latitude): + print(raw_latitude) # 999.0 + case latitude: + print(latitude) +``` + +Use [`Location.get_latitude()`][frequenz.client.common.types.Location.get_latitude] when you need a valid number. It returns the +latitude or raises +[`InvalidLatitudeError`][frequenz.client.common.types.InvalidLatitudeError], +whose [`InvalidLatitudeError.value`][frequenz.client.common.types.InvalidLatitudeError.value] is the raw value. + +Some wrapper types use a dedicated subclass for a value that is invalid, +unspecified, or unrecognized. For a battery, that can be +[`UnspecifiedBattery`][frequenz.client.common.microgrid.electrical_components.UnspecifiedBattery] +or +[`UnrecognizedBattery`][frequenz.client.common.microgrid.electrical_components.UnrecognizedBattery], +alongside a known subtype such as +[`LiIonBattery`][frequenz.client.common.microgrid.electrical_components.LiIonBattery]. +They are all [`Battery`][frequenz.client.common.microgrid.electrical_components.Battery] +values. [`UnrecognizedBattery.type`][frequenz.client.common.microgrid.electrical_components.UnrecognizedBattery.type] keeps the raw type. Likewise, +[`MismatchedCategoryElectricalComponent`][frequenz.client.common.microgrid.electrical_components.MismatchedCategoryElectricalComponent] +is an [`ElectricalComponent`][frequenz.client.common.microgrid.electrical_components.ElectricalComponent] +whose [`MismatchedCategoryElectricalComponent.category`][frequenz.client.common.microgrid.electrical_components.MismatchedCategoryElectricalComponent.category] records the mismatched value. + +```python +from typing import assert_never + +from frequenz.client.common.microgrid.electrical_components import ( + BatteryTypes, + LiIonBattery, + NaIonBattery, + UnrecognizedBattery, + UnspecifiedBattery, +) + + +def describe_battery(battery: BatteryTypes) -> str: + match battery: + case LiIonBattery(): + return "Li-ion" + case NaIonBattery(): + return "Na-ion" + case UnspecifiedBattery(): + return "unspecified" + case UnrecognizedBattery(type=raw_type): + return f"unrecognized: {raw_type}" + case unexpected: + assert_never(unexpected) +``` + +Use `match` with [`assert_never`][typing.assert_never] rather than `isinstance()` chains so a type +checker can keep the cases exhaustive. Invalid data is different from an +unrecognized enum integer: `Invalid*` signals an invariant violation, while an +unrecognized integer is unknown but well-formed. See [enum-or-int +fields](enum-or-int-fields.md) for that forward-compatible case. diff --git a/docs/wrapping-guide/SUMMARY.md b/docs/wrapping-guide/SUMMARY.md new file mode 100644 index 00000000..1c512722 --- /dev/null +++ b/docs/wrapping-guide/SUMMARY.md @@ -0,0 +1,8 @@ +* [Overview](index.md) +* [Organizing a wrapper package](organizing-a-wrapper-package.md) +* [Enums](enums.md) +* [Data types](data-types.md) +* [Validity in the type](validity-in-the-type.md) +* [Conversion functions](conversion-functions.md) +* [Deprecation and compatibility](deprecation-and-compatibility.md) +* [Testing](testing.md) diff --git a/docs/wrapping-guide/conversion-functions.md b/docs/wrapping-guide/conversion-functions.md new file mode 100644 index 00000000..010b2ce4 --- /dev/null +++ b/docs/wrapping-guide/conversion-functions.md @@ -0,0 +1,126 @@ +# Conversion functions + +Conversion functions translate low-level protobuf messages into high-level +Python wrappers. They keep generated types out of public wrapper modules. They +also retain values that a newer server sends or that an invalid message +contains. This page explains how to write one; the conversion functions this +library provides follow the same rules. + +## Keep generated code in conversion packages + +Put a conversion function named `*_from_proto` or `*_to_proto` in +`proto//`, where `` identifies a protobuf API version such +as `v1alpha8`. Add a sibling directory when you support a new protobuf API +version. Public type modules must not import generated protobuf code. They +define the wrapper types that conversion functions use. See +[Organizing a wrapper package](organizing-a-wrapper-package.md) for the package +layout. + +Keep the protobuf type in the conversion function signature and export the +function from its versioned `proto` package. Public wrapper types and their +ordinary constructors must not depend on generated message classes. + +## Delegate enum conversion + +An enum conversion function calls +[`enum_from_proto`][frequenz.client.common.proto.enum_from_proto]. The helper +returns a known wrapper enum member. It returns an unrecognized number as +`int`. Use `allow_invalid=False` only when your code cannot accept an +unrecognized value. This is how [Enums](enums.md) keeps an enum usable with +newer protobuf APIs. + +```python +from frequenz.client.common.metrics import Metric +from frequenz.client.common.proto import enum_from_proto + + +assert enum_from_proto(Metric.AC_POWER_ACTIVE.value, Metric) is Metric.AC_POWER_ACTIVE +assert enum_from_proto(999, Metric) == 999 +``` + +Do not copy this logic into each enum conversion function. Using the helper +keeps unrecognized values consistent across wrapper packages. The one exception +is a protobuf API version that numbers a value differently from the wrapper +enum, which [Enums](enums.md) covers: those conversion functions translate the +numbers themselves. + +The matching `*_to_proto` function needs no helper. The wrapper member value is +the protobuf number, as [Enums](enums.md) requires, so the function wraps that +number in the generated `ValueType`: + +```python +# Excerpt from metrics/proto/v1alpha8/_metric.py, without its docstring. +def metric_to_proto(metric: Metric) -> metrics_pb2.Metric.ValueType: + return metrics_pb2.Metric.ValueType(metric.value) +``` + +Type the parameter as the wrapper enum, not `Metric | int`. A caller that +received an unrecognized number decides what to send back; the conversion +function does not choose for it. + +## Return validity in the type + +When a protobuf message breaks a rule for the wrapper type, return +`X | InvalidX`. The valid subclass gives callers the normal behavior. The +invalid subclass keeps the received fields for diagnosis, recovery, or later +interpretation. For example, +[`Bounds`][frequenz.client.common.metrics.Bounds] and +[`InvalidBounds`][frequenz.client.common.metrics.InvalidBounds] represent valid +and invalid ranges, while +[`DeliveryArea`][frequenz.client.common.grid.DeliveryArea] and +[`InvalidDeliveryArea`][frequenz.client.common.grid.InvalidDeliveryArea] do the +same for delivery-area data. See [Validity in the type](validity-in-the-type.md) +for how to model these types. + +Normal construction must enforce the valid type's rules and reject invalid +values. A conversion function, however, must keep invalid protobuf data by +creating the matching invalid subclass. This gives callers a reliable valid type +without discarding a message the client received. Annotate the return type as +the explicit union, not as the shared `Base*` class, so callers must separate +the two cases; see [Validity in the type](validity-in-the-type.md). + +When working with this result, use a safe `get_*()` accessor if the wrapper has +one. Otherwise, handle every member of the union with `match` and +[`assert_never`][typing.assert_never]: + +```python +from typing import assert_never + +from frequenz.client.common.metrics import Bounds, InvalidBounds + + +def describe(bounds: Bounds | InvalidBounds) -> str: + match bounds: + case Bounds(): + return "valid" + case InvalidBounds(): + return "invalid" + case unexpected: + assert_never(unexpected) + + +assert describe(Bounds(lower=0, upper=1)) == "valid" +assert describe(InvalidBounds(lower=1, upper=0)) == "invalid" +``` + +## Check unset fields and alternatives + +Generated scalar defaults do not tell you whether a field was sent. Use +[`HasField()`][google.protobuf.message.Message.HasField] for an optional or +message field. Use [`WhichOneof()`][google.protobuf.message.Message.WhichOneof] +to check the active option in a `oneof` before you read it. Base the conversion +on those results, not on a default value that could mean the field is absent or +set. + +When a message contains descriptor-known content that the current wrapper type +does not model, retain it in a JSON-compatible mapping for inspection. When +creating that mapping with +[`MessageToDict()`][google.protobuf.json_format.MessageToDict], pass +`preserving_proto_field_name=True` so its keys use the original protobuf field +names. + +This mapping is not a lossless protobuf representation. It does not preserve +unknown wire fields, all field-presence information, or the exact protobuf +representation of each value. Do not use it when the wrapper must write the +original content back unchanged; preserve a lossless representation for that +use case instead. diff --git a/docs/wrapping-guide/data-types.md b/docs/wrapping-guide/data-types.md new file mode 100644 index 00000000..97ef911f --- /dev/null +++ b/docs/wrapping-guide/data-types.md @@ -0,0 +1,110 @@ +# Data types + +Wrapper objects should make their values, identity, and behavior clear. Do not +expose generated protobuf details. Use immutable data classes, typed +identifiers, and numeric annotations that match runtime values. + +## Use immutable, keyword-only value objects + +Declare record-like wrapper types as dataclasses with `frozen=True` and +`kw_only=True`. Freezing prevents fields from being reassigned; use immutable +field values too when the wrapper must be fully immutable. Equality depends on +the field values, and instances are hashable when those values are hashable. +Keyword-only construction keeps calls readable when fields change. Give each +public wrapper a short, useful `__str__` representation. For example, +[`Bounds`][frequenz.client.common.metrics.Bounds] renders its interval without +the extra detail of a dataclass representation. + +```python +# Simplified from metrics/_bounds.py, without its validation. +import dataclasses + +from frequenz.core.typing import FloatInt + + +@dataclasses.dataclass(frozen=True, kw_only=True) +class Bounds: + """A set of lower and upper bounds for any metric.""" + + lower: FloatInt | None = None + """The lower bound, or `None` when unbounded.""" + + upper: FloatInt | None = None + """The upper bound, or `None` when unbounded.""" + + def __str__(self) -> str: + """Return a string representation of these bounds.""" + return f"[{self.lower},{self.upper}]" + + +assert str(Bounds(lower=0, upper=1)) == "[0,1]" +``` + +Keep the string form short enough for log messages and errors. Show the value's +useful identity or state. Do not repeat every internal detail or protobuf field. + +## Use typed identifiers + +Use a small type derived from [`BaseId`][frequenz.core.id.BaseId] for an +identifier. Pass its `str_prefix` as a class argument, and decorate the class +with [`final`][typing.final] because an ID type names one kind of entity and +nothing should subclass it further. + +A typed ID stays integer-like for storage and comparison, and stops unrelated +IDs from comparing equal. For example, +[`MicrogridId`][frequenz.client.common.microgrid.MicrogridId] and another ID +type with the same numeric value are still different values and dictionary keys. + +```python +from typing import final + +from frequenz.core.id import BaseId + + +@final +class MicrogridId(BaseId, str_prefix="MID"): + """A unique identifier for a microgrid.""" + + +assert str(MicrogridId(42)) == "MID42" +``` + +Do not expose a bare `int` when you know what the identifier identifies. A +dedicated ID type tells callers which ID a function needs and gives compact +strings a recognizable prefix. + +## Tell the truth about numbers + +Use [`FloatInt`][frequenz.core.typing.FloatInt], which is exactly +`float | int`, for a value that may be an `int` at runtime even when a type +checker accepts it as `float`. [PEP 484's numeric tower](https://peps.python.org/pep-0484/#the-numeric-tower) +allows an `int` where `float` is annotated. A plain `float` annotation can hide +a runtime case that matters when code checks the concrete number type. + +The alias lives in `frequenz-core`, so every Frequenz library annotates these +values the same way. Import it from there instead of defining a local copy. + +Keep a numeric API generic when it only does arithmetic. When code must check +the concrete number type, handle both possibilities together: + +```python +from typing import assert_never + +from frequenz.core.typing import FloatInt + + +def describe(value: FloatInt) -> str: + match value: + case float() | int(): + return f"number:{value}" + case unexpected: + assert_never(unexpected) + + +assert describe(1) == "number:1" +assert describe(1.5) == "number:1.5" +``` + +Do not match only `float()`. An `int` accepted by the annotation would not +match. `bool` is also an `int` subclass. Reject it explicitly only when the +value you model needs that distinction. diff --git a/docs/wrapping-guide/deprecation-and-compatibility.md b/docs/wrapping-guide/deprecation-and-compatibility.md new file mode 100644 index 00000000..cf0ddb36 --- /dev/null +++ b/docs/wrapping-guide/deprecation-and-compatibility.md @@ -0,0 +1,88 @@ +# Deprecation and compatibility + +Use these steps when you change a public wrapper type or conversion function. +They keep the old name visible and usable for a limited time. The upstream +[semantic-versioning rules](https://github.com/frequenz-floss/docs/blob/v0.x.x/python/semver-0.x.x.md) +define the wider 0.x versioning process. + +## Name the replacement precisely + +Mark the old public symbol with +[`typing_extensions.deprecated`][typing_extensions.deprecated]. Use this exact +message form: `" is deprecated. Use instead."`. Write both +fully qualified names exactly. In the old API documentation, explain any change +to the return type or behavior. A caller should know what to use from the +warning alone. + +This example gives the replacement conversion function a numeric-suffixed name +and checks its warning: + +```python +from pytest import deprecated_call +from typing_extensions import deprecated + + +def thing_from_proto2(value: int) -> str: + return str(value) + + +@deprecated( + "example.thing_from_proto is deprecated. Use example.thing_from_proto2 instead." +) +def thing_from_proto(value: int) -> str: + return thing_from_proto2(value) + + +with deprecated_call( + match="example.thing_from_proto is deprecated. " + "Use example.thing_from_proto2 instead." +): + assert thing_from_proto(3) == "3" +``` + +## Add a new converter when its contract changes + +When a conversion function's arguments or return type change in an incompatible +way, add a new name with a numeric suffix such as `thing_from_proto2`. Keep the +previous name as a deprecated function. The new function has its own stable +signature and can return the current `X | InvalidX` result described in +[Conversion functions](conversion-functions.md). + +The numeric suffix supports the compatibility transition; it is not necessarily +permanent. At the next minor release, follow the upstream +[semantic-versioning rules](https://github.com/frequenz-floss/docs/blob/v0.x.x/python/semver-0.x.x.md) +for removing deprecated versions and, when applicable, restoring the +unsuffixed name while retaining a deprecated alias for the suffixed name. + +For an enum-member change, use +[`deprecated_member`][frequenz.core.enum.deprecated_member]. It keeps the old +member temporarily and warns when code uses it. Document the representation new +code should use. + +## Tighten invariants in stages + +When you tighten a rule, do not always reject old input immediately. First, +accept it and emit a [`DeprecationWarning`][] with +[`warnings.warn()`][warnings.warn]. Where feasible, provide a documented opt-in +flag for the stricter behavior. In a later minor release, make invalid normal +construction raise every time. A conversion function must still keep invalid +protobuf data in the typed invalid result from +[Validity in the type](validity-in-the-type.md). + +This lets callers find affected construction code when warnings are errors. They +can test the stricter behavior before it becomes required and migrate on purpose. + +## Test and document each transition + +Test every public deprecation with +[`pytest.deprecated_call()`][pytest.deprecated_call]. Check the exact message +and the replacement behavior. If deprecated code correctly calls another +deprecated symbol, suppress only that expected inner +[`DeprecationWarning`][] +in a small [`warnings.catch_warnings()`][warnings.catch_warnings] block. The +outer API must still emit its one public warning. + +Add `RELEASE_NOTES.md` migration bullets that state the old behavior, the +replacement, what changes, and the planned removal version. Remove the +deprecated name at the right minor-version bump. Follow the upstream +[semantic-versioning rules](https://github.com/frequenz-floss/docs/blob/v0.x.x/python/semver-0.x.x.md). diff --git a/docs/wrapping-guide/enums.md b/docs/wrapping-guide/enums.md new file mode 100644 index 00000000..581bcbdb --- /dev/null +++ b/docs/wrapping-guide/enums.md @@ -0,0 +1,232 @@ +# Enums + +When you wrap a protobuf enum, first ask what its values mean to callers. Do +not copy it just because it is an enum. A status can become a boolean, a +component category can become a class, and a named vocabulary can remain an +enum. + +## Translate a protobuf enum to a higher-level construct + +Use this when callers need one simple answer, not the enum values themselves. +[`Microgrid`][frequenz.client.common.microgrid.Microgrid] is an example. Its +[`is_active()`][frequenz.client.common.microgrid.Microgrid.is_active] method +returns a `bool` for a known status. + +```python +# `microgrid` was returned by microgrid_from_proto(...), with an ACTIVE status. +assert microgrid.is_active() is True +``` + +### Implementation mechanics + +The private `_active` field stores `bool | int`. It is an implementation detail, +not public API. The conversion function turns the known protobuf status values +into `True` and `False`. It keeps `0` and unrecognized values as their raw +`int` so the public method can report the right problem. + +```python +# Internal excerpt, simplified from microgrid/proto/v1alpha8/_microgrid.py. +_ACTIVE_BY_STATUS = { + MICROGRID_STATUS_ACTIVE: True, + MICROGRID_STATUS_INACTIVE: False, +} + + +def _microgrid_status_to_active(value: int) -> bool | int: + return _ACTIVE_BY_STATUS.get(value, value) + + +# The converter passes this value to the wrapper. +Microgrid(_active=_microgrid_status_to_active(message.status), ...) +``` + +The accessor reads only that stored value. It does not derive the result later. + +```python +# Internal excerpt, simplified from microgrid/_microgrid.py. +def is_active(self) -> bool: + match self._active: + case bool() as active: + return active + case 0: + raise UnspecifiedEnumValueError(...) + case int() as value: + raise UnrecognizedEnumValueError(..., value, ...) + case unknown: + assert_never(unknown) +``` + +Use this when the enum only expresses a higher-level fact such as active or +inactive. Do not use it when callers must retain, compare, or write back each +distinct protobuf value. + +## Make a class hierarchy the type identity + +Use a class hierarchy when a category says what the object is. The electrical +component families are [`Battery`][frequenz.client.common.microgrid.electrical_components.Battery], +[`Inverter`][frequenz.client.common.microgrid.electrical_components.Inverter], +and [`EvCharger`][frequenz.client.common.microgrid.electrical_components.EvCharger]. +Their concrete subclasses carry the subtype: for example, +[`LiIonBattery`][frequenz.client.common.microgrid.electrical_components.LiIonBattery], +[`PvInverter`][frequenz.client.common.microgrid.electrical_components.PvInverter], +or +[`AcEvCharger`][frequenz.client.common.microgrid.electrical_components.AcEvCharger]. + +### Implementation mechanics + +The electrical-component protobuf has a `category` enum and category-specific +information. That nested message has a `oneof` named `kind`. Its options are +`battery`, `ev_charger`, `grid_connection_point`, `inverter`, and +`power_transformer`. Only one option can be set. + +The converter calls [`WhichOneof("kind")`][google.protobuf.message.Message.WhichOneof] +to learn which nested message is set. +It checks that name against the declared `category`. A disagreement creates a +problematic wrapper instead of guessing. The `match` on `category` then chooses +the component family, and the nested `type` value chooses its concrete class. + +```python +# Internal excerpt, simplified from electrical_components/proto/v1alpha8/ +# _electrical_component.py. +kind = message.category_specific_info.WhichOneof("kind") +category = enum_from_proto(message.category, ElectricalComponentCategory) + +if ( + kind + and isinstance(category, ElectricalComponentCategory) + and category.name.lower() != kind +): + return MismatchedCategoryElectricalComponent(...) + +match category: + case ElectricalComponentCategory.BATTERY: + raw_type = message.category_specific_info.battery.type + cls = _BATTERY_CLASS_BY_PROTO_TYPE.get(raw_type) + return cls(...) if cls else UnrecognizedBattery(..., type=raw_type) + + case ElectricalComponentCategory.EV_CHARGER: + raw_type = message.category_specific_info.ev_charger.type + cls = _EV_CHARGER_CLASS_BY_PROTO_TYPE.get(raw_type) + return cls(...) if cls else UnrecognizedEvCharger(..., type=raw_type) + + case ElectricalComponentCategory.INVERTER: + raw_type = message.category_specific_info.inverter.type + cls = _INVERTER_CLASS_BY_PROTO_TYPE.get(raw_type) + return cls(...) if cls else UnrecognizedInverter(..., type=raw_type) +``` + +[`WhichOneof()`][google.protobuf.message.Message.WhichOneof] does not choose a +class by itself. It identifies the protobuf field and verifies it matches +`category`. The category and subtype lookup then select one concrete subclass +for the returned object. + +Use this when each category has different fields, behavior, or likely future +extensions. Do not use it for a small, stable vocabulary whose values need no +object-specific data or behavior. + +## Export a semantic enum as-is + +Use an enum when its named members are the public concept. For example, +[`Metric`][frequenz.client.common.metrics.Metric] gives callers a stable list of +metric names and numbers. Copy the known names and numeric values, but do not +copy an `UNSPECIFIED` member. + +Derive each member name from the protobuf value name by removing the fixed +prefix that repeats the enum name, so `METRIC_DC_VOLTAGE` becomes `DC_VOLTAGE`. +Keep the protobuf number as the member value, so conversion stays a plain +lookup. Subclass [`Enum`][frequenz.core.enum.Enum] from `frequenz-core` instead +of the standard library enum, because it supports the member deprecation +described in [Deprecation and compatibility](deprecation-and-compatibility.md). +Decorate the class with [`unique`][frequenz.core.enum.unique], which rejects two +non-deprecated members sharing a number. Give each member a one-line docstring. + +```python +# A simplified wrapper enum. Known numeric values match the protobuf values. +from frequenz.core.enum import Enum, unique + + +@unique +class Metric(Enum): + """List of supported metrics.""" + + DC_VOLTAGE = 1 + """The DC voltage.""" + + DC_CURRENT = 2 + """The DC current.""" + + AC_POWER_ACTIVE = 26 + """The AC active power.""" +``` + +The parity test described in [Testing](testing.md) checks every wrapper member +against its generated name and number, so a member that drifts from its protobuf +value fails the test suite. It intentionally accepts generated values that the +wrapper does not expose yet, preserving compatibility when the protobuf API adds +a value before the wrapper does. + +!!! warning "Renumbered protobuf values" + + When a new protobuf API version keeps a value name but changes its number, + give the member the number from the newest version you support. No single + wrapper enum can match both versions, and taking the newest number keeps + conversion for that version a plain lookup, with no mapping to maintain on + the path most callers use. + + The `proto//` package for the older version must then translate + its numbers explicitly instead of delegating to + [`enum_from_proto`][frequenz.client.common.proto.enum_from_proto]. It also + needs its own tests in place of the parity test, which no longer holds for + those numbers. + + Renumbering a member is a breaking change for callers that read `.value`, + so release it with the support for the new protobuf API version and record + it as described in + [Deprecation and compatibility](deprecation-and-compatibility.md). + +The shared enum helper first tries to create the wrapper enum. If the number is +not a member, it returns the raw `int` instead. That preserves values added by a +newer protobuf API. + +```python +# Internal excerpt, simplified from proto/_enum.py. +def enum_from_proto(value: int, enum_type: type[EnumT]) -> EnumT | int: + try: + return enum_type(value) + except ValueError: + return value +``` + +An exported wrapper enum has no `UNSPECIFIED` member. The protobuf value `0` is +the plain `int` `0`. Type every enum-valued field as `TheEnum | int`. The `int` +case covers both `0` and an unrecognized nonzero value. + +When consuming such a field directly, match all three cases. Put `case 0` +before `case int()`, then finish with [`assert_never`][typing.assert_never]. + +```python +from typing import assert_never + +from frequenz.client.common.metrics import Metric + + +def describe(metric: Metric | int) -> str: + match metric: + case 0: + return "unspecified" + case Metric() as known: + return known.name + case int() as value: + return f"unrecognized:{value}" + case unexpected: + assert_never(unexpected) + + +assert describe(Metric.AC_POWER_ACTIVE) == "AC_POWER_ACTIVE" +assert describe(0) == "unspecified" +assert describe(999) == "unrecognized:999" +``` + +Use this when callers naturally compare, select, or display named values. Do +not use it merely because the protobuf uses an enum. Use a boolean or class +hierarchy when either describes the Python type more clearly. diff --git a/docs/wrapping-guide/index.md b/docs/wrapping-guide/index.md new file mode 100644 index 00000000..a196faeb --- /dev/null +++ b/docs/wrapping-guide/index.md @@ -0,0 +1,36 @@ +# Wrapping Guide + +This guide explains how to build safe, idiomatic Python wrappers out of the +low-level protobuf message bindings generated by protobuf/gRPC, along with the +repository conventions that keep those wrappers coherent. Callers work with +stable Python types. The conversion functions keep generated protobuf details +inside the versioned packages that need them. + +Use this guide when you add a wrapper or change an existing one. It starts with +package structure, then covers enums, data types, validity, and conversion +functions. It ends with the tests that keep those choices working. The +[User Guide](../user-guide/index.md) explains how callers use the wrapper +types, and the [Client Developer Guide](../client-developer-guide/index.md) +explains how client libraries call conversion functions and return the +wrappers. + +## Sections + +- [Organizing a wrapper package](organizing-a-wrapper-package.md) — Explains + where public wrapper types and version-specific conversion functions belong. + Use it to keep generated protobuf imports out of public type modules. +- [Enums](enums.md) — Shows when a protobuf enum should become a boolean, a + class hierarchy, or a public Python enum. It also shows how to keep + unrecognized values. +- [Data types](data-types.md) — Covers immutable wrapper objects, typed + identifiers, and numeric annotations that match Python's runtime behavior. +- [Validity in the type](validity-in-the-type.md) — Explains how to represent + valid, invalid, missing, and unknown values without dropping received data. +- [Conversion functions](conversion-functions.md) — Explains how to write the + `*_from_proto` and `*_to_proto` functions that translate protobuf messages + to wrapper types. +- [Deprecation and compatibility](deprecation-and-compatibility.md) — Describes + how to replace public functions and tighten validation without surprising + callers. +- [Testing](testing.md) — Shows how to place tests, check enum parity, and make + documentation examples and warnings part of the test suite. diff --git a/docs/wrapping-guide/organizing-a-wrapper-package.md b/docs/wrapping-guide/organizing-a-wrapper-package.md new file mode 100644 index 00000000..65d0be73 --- /dev/null +++ b/docs/wrapping-guide/organizing-a-wrapper-package.md @@ -0,0 +1,62 @@ +# Organizing a wrapper package + +A wrapper package keeps public Python types separate from the code that reads +and writes generated protobuf messages. Callers can import and type-check the +wrapper types without depending on generated bindings. The conversion functions +know which protobuf API version they support. + +## Put types and converters in separate layers + +Give each group of wrapper types a public package. Put its conversion functions +in a versioned `proto//` subpackage. Public type modules must not +import generated protobuf modules. Only conversion modules may import them. + +```text +/ +├── __init__.py # public re-exports and sorted __all__ +├── _.py # public type in a private implementation module +└── proto/ + └── v1alpha8/ + ├── __init__.py # public converter re-exports and sorted __all__ + └── _.py # generated-message conversion implementation +``` + +Use a sibling directory such as `proto/v1alpha8/` for each protobuf API version. +When support for a new version is needed, add another sibling directory. Do not +change the existing one. This lets one set of public wrapper types support +several protobuf API versions without exposing generated types. + +## Make the package initializer the public surface + +Put public types in underscore-prefixed implementation modules and re-export +them from the package initializer. Define an alphabetically sorted `__all__` in +that initializer. This gives callers stable, easy-to-find imports without +making implementation-module paths public. Follow the same rule in each +versioned conversion package. + +Internal modules use relative imports from the module that defines a symbol. +Callers import only package-level names, such as +[`Metric`][frequenz.client.common.metrics.Metric], not an underscore module. +This avoids import cycles and makes the supported API clear. + +## Name conversion functions by direction and protobuf API version + +Name a conversion function `_from_proto` or `_to_proto`. Export +it from the matching `proto//` package. The import path, not the +function name, shows which protobuf API version it supports. A `*_from_proto` +function returns a public wrapper type. It can return a documented invalid +wrapper when it must retain malformed protobuf data. + +## Preserve semantics in field names and docstrings + +Match wrapper field names to protobuf field names by default. Diverge only when +a clear Pythonic improvement preserves or clarifies the field's meaning. Keep +the `_id` suffix for identifiers, even when you drop a redundant entity prefix. +For example, `source_electrical_component_id` can become `source_id`. Use +`_time` for protobuf `_time` and `_timestamp` fields, so names such as +`create_time` stay clear. + +In a type whose name ends in `Value`, you may drop a redundant `_value` suffix +if the remaining name stays clear. Docstrings may be shorter or more Pythonic +than generated comments. They must not narrow, broaden, contradict, or change +the meaning of the protobuf field. diff --git a/docs/wrapping-guide/testing.md b/docs/wrapping-guide/testing.md new file mode 100644 index 00000000..d186c291 --- /dev/null +++ b/docs/wrapping-guide/testing.md @@ -0,0 +1,48 @@ +# Testing + +Tests check that public Python wrappers keep working with generated protobuf +messages. Mirror the source layout, reuse the enum-parity helper, and check +warnings explicitly so the suite catches unwanted API changes early. + +## Mirror the source tree + +Mirror `src/frequenz/client/common/` under `tests/`, without that prefix. For +example, `src/frequenz/client/common/metrics/_thing.py` normally becomes +`tests/metrics/test_thing.py`. Drop the leading implementation underscore and +add the `test_` prefix. Keep the `proto//` directories. Split a +large source module into a matching test subdirectory when its types or +functions need separate files. + +Use absolute public imports in tests. This checks that supported imports work. +It also stops a test from relying on an internal implementation-module path. + +## Reuse enum parity tests + +For an exported enum that matches a protobuf enum, subclass +[`EnumParityTest`][frequenz.client.common.test.enum_parity.EnumParityTest]. Set +its `python_enum`, `proto_enum`, `name_prefix`, `from_proto`, and `to_proto` +attributes. The inherited tests check wrapper member names and numbers, +known-value conversion, and unknown values. They intentionally accept generated +protobuf members that the wrapper does not expose yet, so a newer protobuf API +remains compatible with an older wrapper. + +A protobuf API version that numbers a value differently from the wrapper enum +cannot use this helper, because the parity checks compare those numbers. Write +explicit tests for that version instead, pinning each generated value to the +member it converts to. [Enums](enums.md) explains when this happens. + +## Treat documentation and warnings as tests + +[Sybil](https://sybil.readthedocs.io/) collects Python examples in source +docstrings and checks that they pass some basic linter checks. Keep each example +self-contained and correct for its API. The test suite treats most warnings as +errors; deprecation warnings are configured separately and should be asserted +explicitly. + +Assert each expected deprecation with +[`pytest.deprecated_call()`][pytest.deprecated_call]. This records the public +warning and stops an unrelated warning from being hidden. When deprecated code +correctly calls another deprecated symbol, suppress only that inner +`DeprecationWarning` in a small warning block to avoid duplicate messages. In a +test, use the same small suppression only for a warning that a dedicated +assertion already checks. Never suppress warnings globally. diff --git a/docs/wrapping-guide/validity-in-the-type.md b/docs/wrapping-guide/validity-in-the-type.md new file mode 100644 index 00000000..0fe58953 --- /dev/null +++ b/docs/wrapping-guide/validity-in-the-type.md @@ -0,0 +1,122 @@ +# Validity in the type + +Invalid protobuf data is still data the client received. Show whether it is +valid in the wrapper type. This lets a conversion function keep the received +data, ordinary construction enforce its rules, and later code ask for a valid +value when it needs one. For guidance on using these types, see the User Guide's +[validity-in-the-type section](../user-guide/validity-in-the-type.md) and +[safe-accessor section](../user-guide/safe-accessors.md). + +## Model valid and invalid object states + +For an object with rules that cover the whole object, define a guarded `Base*` +class and two concrete subclasses: the valid type and its `Invalid*` +counterpart. The base holds shared fields. Its `__new__` guard prevents callers +from constructing the base directly, so each instance has a meaningful state. +For example, +[`DeliveryArea`][frequenz.client.common.grid.DeliveryArea] and +[`InvalidDeliveryArea`][frequenz.client.common.grid.InvalidDeliveryArea] share +a base while making their validity visible in annotations. + +Normal constructors enforce the valid subclass's rules. A conversion function +that sees invalid protobuf data creates the matching invalid subclass and +returns `X | InvalidX`. The invalid subclass keeps the raw fields for +diagnosis, recovery, and later interpretation. It does not invent a valid +value. + +Annotate results and fields with the union `X | InvalidX`, never with the +guarded base class. The union tells callers that invalid data is possible, +forces them to separate the two cases — or lets them require only the valid +type where nothing else makes sense — and keeps a `match` over the result +exhaustive. A `Base*` annotation would hide which states exist and accept any +future subclass, so a type checker could not check either decision. For +example, a delivery-area conversion function returns +[`DeliveryArea`][frequenz.client.common.grid.DeliveryArea]` | `[`InvalidDeliveryArea`][frequenz.client.common.grid.InvalidDeliveryArea], +and +[`MetricSample.bounds_set`][frequenz.client.common.metrics.MetricSample.bounds_set] +is annotated +[`BoundsSet`][frequenz.client.common.metrics.BoundsSet]` | `[`InvalidBoundsSet`][frequenz.client.common.metrics.InvalidBoundsSet], +while [`BaseDeliveryArea`][frequenz.client.common.grid.BaseDeliveryArea] and +[`BaseBounds`][frequenz.client.common.metrics.BaseBounds] appear in no public +signature. + +## Wrap invalid fields precisely + +An otherwise valid object can have one invalid field. In that case, put a +field-specific `Invalid*` wrapper in that field's union. Do not mark unrelated +data invalid. [`Location`][frequenz.client.common.types.Location] does this +with [`InvalidLatitude`][frequenz.client.common.types.InvalidLatitude], +[`InvalidLongitude`][frequenz.client.common.types.InvalidLongitude], and +[`InvalidCountryCode`][frequenz.client.common.types.InvalidCountryCode]. Each +wrapper keeps the raw value while leaving the other fields usable. + +## Represent protobuf recovery as a subtype + +When the class identifies a protobuf category or type, use dedicated subclasses +for recovery cases. An +[`UnspecifiedBattery`][frequenz.client.common.microgrid.electrical_components.UnspecifiedBattery] +represents a missing type, an +[`UnrecognizedBattery`][frequenz.client.common.microgrid.electrical_components.UnrecognizedBattery] +retains an unknown raw type, and +[`MismatchedCategoryElectricalComponent`][frequenz.client.common.microgrid.electrical_components.MismatchedCategoryElectricalComponent] +records conflicting category information. These subclasses make the recovery +case clear without pretending it is a known component. + +Use the complete public type alias when working with guarded bases and recovery +subclasses. This handler deliberately does not construct an object. Type-check +it instead of running it: + +```python +from typing import assert_never + +from frequenz.client.common.microgrid.electrical_components import ( + BatteryTypes, + LiIonBattery, + NaIonBattery, + UnrecognizedBattery, + UnspecifiedBattery, +) + + +def describe_battery(battery: BatteryTypes) -> str: + match battery: + case LiIonBattery(): + return "li-ion" + case NaIonBattery(): + return "na-ion" + case UnspecifiedBattery(): + return "unspecified" + case UnrecognizedBattery(type=raw_type): + return f"unrecognized:{raw_type}" + case unexpected: + assert_never(unexpected) +``` + +## Design semantic accessors and errors together + +Keep the low-level field unchanged, then add a `get_*()` accessor for callers +that need a valid value. Use an exhaustive `match` for valid, invalid, missing, +and unknown cases. End it with [`assert_never`][typing.assert_never]. For +example, [`Location.get_latitude()`][frequenz.client.common.types.Location.get_latitude] +returns a validated number and raises a typed error for an invalid latitude +wrapper. + +Use this error hierarchy: +[`ClientCommonError`][frequenz.client.common.ClientCommonError] → +[`InvalidAttributeError`][frequenz.client.common.InvalidAttributeError] +(also a `ValueError`) → +[`UnspecifiedEnumValueError`][frequenz.client.common.UnspecifiedEnumValueError], +[`UnrecognizedEnumValueError`][frequenz.client.common.UnrecognizedEnumValueError], +[`MissingFieldError`][frequenz.client.common.MissingFieldError], and +domain-specific invalid-value errors such as +[`InvalidBoundsSetError`][frequenz.client.common.metrics.InvalidBoundsSetError]. +Keep a specific error's raw value or invalid object on the error instance so +the caller can inspect it. + +## Make invalid string output easy to search + +Use the `` marker only for a failed rule, such as a malformed +required field or the unspecified raw value `0`. It makes real failures clear +in logs and diagnostics. For data that is merely unknown to this version, show +the raw value as `:field=value`. Unknown data does not itself break a rule. +Use these compact forms consistently in each wrapper's `__str__` method. diff --git a/mkdocs.yml b/mkdocs.yml index 510ae0f2..73a4c8d5 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -122,7 +122,9 @@ plugins: # See https://mkdocstrings.github.io/python/usage/#import for details - https://docs.python.org/3/objects.inv - https://docs.pytest.org/en/stable/objects.inv + - https://frequenz-floss.github.io/frequenz-api-common/v0.8/objects.inv - https://frequenz-floss.github.io/frequenz-core-python/v1/objects.inv + - https://googleapis.dev/python/protobuf/latest/objects.inv - https://typing-extensions.readthedocs.io/en/stable/objects.inv # Note this plugin must be loaded after mkdocstrings to be able to use macros # inside docstrings. diff --git a/src/frequenz/client/common/AGENTS.md b/src/frequenz/client/common/AGENTS.md index c8981b55..1cf00fcb 100644 --- a/src/frequenz/client/common/AGENTS.md +++ b/src/frequenz/client/common/AGENTS.md @@ -28,7 +28,8 @@ for downstream users, they don't wrap protobuf messages. ## CORE RULES -- **Wrapper field names and docstrings follow the rules in [CONTRIBUTING.md](../../../../CONTRIBUTING.md).** +- **Wrapper field names and docstrings follow the rules in + [`organizing-a-wrapper-package.md`](../../../../docs/wrapping-guide/organizing-a-wrapper-package.md).** - **Public symbols live in `_name.py`, exported via the package `__init__.py`.** External importers never use the underscore module path. - **Internal cross-module imports are ALWAYS relative and use the real symbol @@ -41,36 +42,28 @@ for downstream users, they don't wrap protobuf messages. must NOT import `protobuf` / `frequenz.api.common`; only the `proto/` modules may. - **New API namespace = new sibling dir** `proto/v1alphaN/`. Never edit `v1alpha8` in place; there is no shared/unversioned converter module. -- Converter naming: `_from_proto(message) -> T | int` and `_to_proto(T) -> ...ValueType`. - Richer parsers use the `_from_proto_with_issues` suffix (returns value + collected issues). - -## ENUMS (most common case) - -- Python enum mirrors a protobuf enum: member name = proto name minus a fixed prefix - (e.g. `METRIC_` → `Metric`), member value = proto numeric value. Start with `UNSPECIFIED = 0`. -- Decorate with `@enum.unique`; one-line `"""docstring"""` under each member. -- Conversion delegates to the shared helper — do NOT reimplement: - ```python - from ....proto import enum_from_proto - def metric_from_proto(message): return enum_from_proto(message, Metric) - def metric_to_proto(metric): return metrics_pb2.Metric.ValueType(metric.value) - ``` -- `enum_from_proto` (`proto/_enum.py`) returns the member for known values, raw `int` for - unknown ones (forward-compat). `allow_invalid=False` raises instead. - -## NON-ENUM TYPES - -- Use `@dataclass(frozen=True, kw_only=True)`; give a custom `__str__` for compact display. -- Fields that may carry an unknown proto enum are typed `T | int` (see `MetricSample.metric`). -- IDs subclass `frequenz.core.id.BaseId` with a `str_prefix=` and are `@final`. - -## DEVIATIONS (do not "fix" blindly) - -- Two issue-reporting styles coexist: `*_with_issues` returns issues; `location_from_proto` - logs a `warning` and silently clamps out-of-range values. Match the neighbor you edit. +- Converter naming: `_from_proto(message)` and `_to_proto(value)`. A converter that + preserves malformed message data returns a typed `X | InvalidX`; enum converters follow the + enum-or-int rule from the guides below. Do not add issue side channels. + +## DESIGN PATTERNS → USE THE GUIDES (don't duplicate here) + +The *how and why* of wrapper/converter design lives in `docs/wrapping-guide/` +(authored, example-backed). When adding or changing a wrapper or converter, follow the +relevant page instead of re-deriving it — and keep the code consistent with it: + +| Task | Guide page | +|------|-----------| +| Package/module layout, proto isolation | [`organizing-a-wrapper-package.md`](../../../../docs/wrapping-guide/organizing-a-wrapper-package.md) | +| Enum representation (bool / class hierarchy / export-as-is); member naming & docstrings; no `UNSPECIFIED`; `TheEnum \| int` | [`enums.md`](../../../../docs/wrapping-guide/enums.md) | +| Frozen kw-only dataclasses + `__str__`, typed IDs (`BaseId`, `str_prefix`, `@final`), `FloatInt` | [`data-types.md`](../../../../docs/wrapping-guide/data-types.md) | +| Validity in the type (`X \| InvalidX`, never `Base*`; per-field `Invalid*`; recovery subtypes) | [`validity-in-the-type.md`](../../../../docs/wrapping-guide/validity-in-the-type.md) | +| Writing `*_from_proto` / `*_to_proto`; delegating to `enum_from_proto`; `HasField`/`WhichOneof`; preserving raw wire data | [`conversion-functions.md`](../../../../docs/wrapping-guide/conversion-functions.md) | +| Deprecation & compatibility | [`deprecation-and-compatibility.md`](../../../../docs/wrapping-guide/deprecation-and-compatibility.md) | +| Testing (`EnumParityTest`, Sybil) | [`testing.md`](../../../../docs/wrapping-guide/testing.md) | ## DON'T - No `as any`-style escapes / `# type: ignore` to silence mypy strict. -- Don't import protobuf-generated modules from pure-type modules. +- Don't import protobuf-generated modules from pure-type modules (only `proto/` may). - Don't add a public symbol without adding it to the domain `__init__.py` `__all__`. diff --git a/src/frequenz/client/common/proto/_enum.py b/src/frequenz/client/common/proto/_enum.py index 810ca8e7..2c971c7a 100644 --- a/src/frequenz/client/common/proto/_enum.py +++ b/src/frequenz/client/common/proto/_enum.py @@ -29,19 +29,21 @@ def enum_from_proto( Example: ```python - import enum + from frequenz.core.enum import Enum, unique from proto import proto_pb2 # Just an example. pylint: disable=import-error - @enum.unique - class SomeEnum(enum.Enum): + @unique + class SomeEnum(Enum): # These values should match the protobuf enum values. - UNSPECIFIED = 0 SOME_VALUE = 1 enum_value = enum_from_proto(proto_pb2.SomeEnum.SOME_ENUM_SOME_VALUE, SomeEnum) # -> SomeEnum.SOME_VALUE + enum_value = enum_from_proto(0, SomeEnum) + # -> 0 + enum_value = enum_from_proto(42, SomeEnum) # -> 42 diff --git a/tests/AGENTS.md b/tests/AGENTS.md index c924c506..61bba136 100644 --- a/tests/AGENTS.md +++ b/tests/AGENTS.md @@ -3,6 +3,10 @@ Tests verify that the idiomatic wrappers stay in lock-step with the generated `frequenz.api.common.*_pb2` bindings they wrap (parity + round-trip conversion). +Testing *philosophy* — `EnumParityTest`, Sybil docstring examples, warnings-as-errors — is in +[`docs/wrapping-guide/testing.md`](../docs/wrapping-guide/testing.md). This file is the +repo-specific **layout + naming** mechanics that the guide does not cover. + ## LAYOUT Tests mirror the package tree with the `src/frequenz/client/common/` prefix stripped.