Adopt the frequenz-core deprecation tooling by llucax · Pull Request #288 · frequenz-floss/frequenz-client-common-python

llucax · 2026-09-21T12:47:16Z

Gets the deprecation tooling in this repo up to date before the 0.4.1 release: deprecation notices that actually reach the API reference, and warnings that stop repeating downstream.

Warning

This depends on frequenz-floss/frequenz-core-python#200 (stacked on #199), which isn't released yet. The WIP: commit pins frequenz-core to that PR's head, and has to be replaced with frequenz-core >= 1.5.0, < 2 before merging.

The docs side styles the Deprecated admonition and generates it automatically: griffe-warnings-deprecated covers symbols decorated with @deprecated, and griffe-frequenz-core covers enum members wrapped in deprecated_member() and module aliases built with deprecated_aliases(). Every deprecation message in the repo then follows the deprecations guide, and hand-written admonitions stay only where nothing can generate one. The commit message lists those cases.

The code side replaces the 16 warnings.catch_warnings() blocks the library used to silence its own deprecations with frequenz.core.warnings.ignoring_deprecations(). Entering and leaving a catch_warnings() block resets the deduplication history of the whole program (python/cpython#73858), so every warning already shown, from any code, was shown again on each converter call: calling two deprecated converters 1,000 times showed 3,000 warnings instead of 3. Two regression tests cover it. The tests use ignoring_deprecations() and asserting_no_deprecations() too, and the wrapping guide now explains both helpers and deprecated_aliases().

Worth a reviewer's eye:

MetricSample.sample_time recommends get_sample_time() rather than sample_time2, because that's its exact replacement (same type, same error for a malformed timestamp). The failing test on the previous push expected sample_time2; the test was the wrong side.
The ElectricalComponentCategory members keep their hand-written admonitions and get no deprecated label, because their message comes from a helper function the extension can't read. Writing 20 long literal messages out seemed worse.
The runtime messages of the DeliveryArea construction deprecations are unchanged. The guide's message form is for the decorator, and downstream tests may match those texts.

Each alias in the deprecated_aliases() example in the wrapping guide now gives its own since, the version it is deprecated in, using the DeprecatedAlias form frequenz-floss/frequenz-core-python#200 added on top of #199, so aliases deprecated in different releases can each say theirs. message is still there as the escape hatch for a custom wording.

llucax · 2026-09-21T12:48:28Z

Example of how it looks.

llucax · 2026-09-21T16:59:25Z

Updated: pushed 3013200 and rewrote the description.

The PR originally only added the styling and deliberately left existing notices alone, which meant it landed a styling rule that almost nothing in this repo used. It now also brings every deprecation in line with the deprecations guide, which turned out to be most of them:

The 26 enum members using frequenz.core.enum.deprecated_member said nothing at all in the docs. The helper predates PEP 702, so griffe generates no admonition for it. They now have hand-written ones, matching what client-microgrid already does.
MetricSample.sample_time and .bounds carry the decorator, but griffe does not see it underneath @property, so they generated nothing either.
The delivery-area code field and invalid DeliveryArea construction used Warning: admonitions, which render as ordinary warnings.
The nine Warning: Deprecated blocks on symbols the decorator does mark are removed, since the extension now generates those and a hand-written copy would show the notice twice. Their extra explanation stays as prose in the docstring body.
The decorator messages are rewritten to the guide's form: fully qualified plain names, the replacement as a bare [name][] cross-reference, and "since v0.4.1" in the sentence.

That takes the rendered count from 0 to 43.

Two things I would like a second opinion on:

ElectricalComponentCategory now spells its decorator message out as a literal instead of reusing the module constant. griffe renders the admonition only when it can read the message statically, so built from a module-level name it produced nothing. The duplication is the price of the notice appearing at all.
docs/wrapping-guide/deprecation-and-compatibility.md was updated to describe the new style.

Two tests asserted a full message and were updated; the rest match a substring. nox is green (2052 tests, mypy, pylint, flake8/pydoclint, black, isort) and the strict docs build passes.

cmd:skip-release-notes is applied: docstrings and warning text change, the API does not.

Copilot

Copilot review overview

🟡 Changes recommended

Decorated properties currently render duplicate notices, and several messages and tests do not follow the newly documented exact format.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 4 · 1

Open (5)

Remove duplicate hand-written deprecation notice · New
Move returned-type guidance into the docstring · New
Remove extra guidance from decorator warning · New
Strengthen warning test to verify replacement link · New
Correct generated admonition placement documentation · New

What changed in this PR

Adds consistent deprecation notices to generated API documentation.

Changes:

Adds and styles generated Deprecated admonitions.
Standardizes deprecation messages and documentation.
Updates warning-message tests.

File	Description
`pyproject.toml`	Adds the Griffe deprecation extension.
`mkdocs.yml`	Configures generated deprecation admonitions.
`docs/_css/mkdocstrings.css`	Styles deprecated notices.
`docs/wrapping-guide/deprecation-and-compatibility.md`	Documents the new convention.
`src/frequenz/client/common/streaming/_event.py`	Documents enum-member deprecation.
`src/frequenz/client/common/proto/_datetime.py`	Updates converter deprecation.
`src/frequenz/client/common/pagination/proto/v1alpha8/_pagination_info.py`	Updates converter deprecation.
`src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_category.py`	Updates legacy converter notices.
`src/frequenz/client/common/microgrid/electrical_components/_state_code.py`	Documents enum-member deprecation.
`src/frequenz/client/common/microgrid/electrical_components/_diagnostic_code.py`	Documents enum-member deprecation.
`src/frequenz/client/common/microgrid/electrical_components/_category.py`	Documents category and member deprecations.
`src/frequenz/client/common/microgrid/components/__init__.py`	Updates `ComponentId` deprecation.
`src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py`	Updates legacy converter notices.
`src/frequenz/client/common/metrics/proto/v1alpha8/_bounds.py`	Updates bounds converter notices.
`src/frequenz/client/common/metrics/_sample.py`	Updates property and enum deprecations.
`src/frequenz/client/common/metrics/_metric.py`	Documents enum-member deprecation.
`src/frequenz/client/common/grid/proto/v1alpha8/_delivery_area.py`	Updates converter deprecation.
`src/frequenz/client/common/grid/_delivery_area.py`	Documents delivery-area deprecations.
`tests/proto/test_datetime.py`	Updates exact warning assertion.
`tests/pagination/proto/v1alpha8/test_pagination_info.py`	Updates exact warning assertion.
`tests/metrics/test_sample_metric_sample.py`	Updates property warning assertion.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

daniel-zullo-frequenz

LGTM, there are a few comments from Copilot but I think most of them can be ignored. I'll approve it again once you've assessed/resolved the existing comments

llucax · 2026-09-22T13:54:50Z

Updated, but I'm exploring an mkdocs extension so some of the fixes here are not necessary (like the duplicated deprecated_member() in enums requiring a duplicate Deprecated: admonition for the docs.

Style the `deprecated` admonition class like a warning, but with the `material/grave-stone` icon and its own colour, so deprecation notices read as deprecation notices and not as generic warnings. That class is what a hand-written `Deprecated:` admonition in a docstring produces, and also what the griffe extension added next will emit, so a single rule covers both sources. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Wire two griffe extensions into the mkdocstrings handler options, so deprecated symbols get a "Deprecated" admonition and a `deprecated` label in the API reference with no docstring edit at all: * `griffe-warnings-deprecated` handles every class and function decorated with `typing_extensions.deprecated`. * `griffe-frequenz-core` handles what `frequenz-core` deprecates through a call instead of a decorator: enum members wrapped in `frequenz.core.enum.deprecated_member()`, and module aliases built with `frequenz.core.warnings.deprecated_aliases()`. It doesn't import `frequenz-core`, so it only goes in the docs dependencies. Both extensions have `kind` set to `deprecated` rather than their default, so they emit `class="deprecated"`, which is exactly what a hand-written `Deprecated:` admonition produces. The CSS rule added in the previous commit then styles all three, and they are visually indistinguishable. That matters because some cases still need the admonition written by hand: a single function argument, a whole module, a property (griffe-warnings-deprecated doesn't look at the decorators of a property), and a message the extensions can't read statically, such as one built by calling a function. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Rewrite every `@deprecated` message in the form the guide asks for: the fully qualified name of the deprecated symbol, the version it was deprecated in, and the replacement as a bare `[name][]` cross-reference, so the admonition generated from it links to the replacement. The old `Warning: Deprecated` admonitions go away, since the generated ones replace them; whatever they said beyond "use X instead" stays as prose in the docstring body. The `deprecated_member()` messages of the `UNSPECIFIED` members follow the same form, and get their admonition generated too. Where no extension can produce one, a `Deprecated:` admonition is written by hand instead: * The members of `ElectricalComponentCategory`, whose message is built by a helper function, which the extension can't read statically. * `MetricSample.sample_time` and `MetricSample.bounds`, because griffe-warnings-deprecated doesn't look at the decorators of a property. * The `bounds` argument of `MetricSample` and a `None` `code` in `DeliveryArea`, which deprecate an argument and not a symbol. * Constructing an invalid `DeliveryArea`, which is being made stricter. `MetricSample.sample_time` now points at `get_sample_time()`, not at `sample_time2`: the property returns a `datetime` and raises `InvalidDatetimeError` for a malformed timestamp, which is exactly what `get_sample_time()` does, while `sample_time2` has a different type and is itself going away when it is renamed back to `sample_time`. The release notes now say the same. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

The previous commits changed every deprecation notice in the repo but left the wrapping guide describing the old message form, `"<old FQCN> is deprecated. Use <new FQCN> instead."`, with no version and no cross-reference brackets. The guide is what a contributor reads before writing a new deprecation, so leaving it behind would reintroduce the old form. It now describes the form the code uses and why: the version in the sentence because a separate "since" line cannot be expressed through the decorator, the replacement as a bare `[name][]` cross-reference so the generated admonition links to it, no backticks because the same string is printed as a runtime warning, and implicit concatenation of single-line strings because a triple-quoted message carries its indentation into both the cross-reference and the terminal. The worked example is updated to match, and now escapes the brackets and dots in its `pytest.deprecated_call()` regex. That is easy to get wrong silently, because `match` is a search and an unescaped `[...]` is a character class that still matches something. It also explains where the admonition comes from, and so why the message has to be made of string literals: the documentation build reads it without running the code, and renders nothing for a message it can't read. The cases that still need a hand-written `Deprecated:` admonition are listed, since the guide never mentioned them: an argument, construction being made stricter, a property, and a message that isn't a literal. The `deprecated_member` paragraph now says its message gets the same treatment, with an example. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

The next commits use `frequenz.core.warnings`, which is not released yet: the module itself is added by frequenz-floss/frequenz-core-python#199, and the per-alias `DeprecatedAlias` messages used here come from frequenz-floss#200, stacked on frequenz-floss#199. Until both are merged and released, depend on frequenz-floss#200's head directly. The dependency goes through the upstream repository URL rather than the fork the PR comes from, because the fork has no tags: built from there, the package gets version `0.0.postN`, while from upstream it is `1.4.0.post34`, which keeps any other `frequenz-core >= 1.4` requirement in the dependency tree satisfiable. This commit must be replaced with `frequenz-core >= 1.5.0, < 2` once v1.5.0 is released, before merging. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

The library silences the deprecation warnings it raises itself, for example when a converter touches a deprecated `UNSPECIFIED` member, with a `warnings.catch_warnings()` block plus an "ignore" filter. Entering and leaving such a block invalidates the deduplication history of every module (python/cpython#73858), so every warning that was already shown, from this library or from anywhere else, is shown again after each call. Downstream this turned into tens of thousands of repeated warnings in a single test run. Replace all 16 blocks with `frequenz.core.warnings.ignoring_deprecations()`, which adds the filter without touching the history. The blocks keep their current extent, so exactly the same warnings are silenced as before; narrowing some of them is a separate change. The `EnumParityTest` scaffold gets the same treatment: it ships in the package and runs inside downstream test suites. Two regression tests call a deprecated converter and a `str()` that silences a deprecation in a loop, under the "default" action, and check that each warning is shown only once. Both fail with the old blocks. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

The tests used `warnings.catch_warnings()` blocks for two different things, now done by the `frequenz.core.warnings` helper made for each: * An "ignore" filter, to call deprecated APIs the test needs as setup, becomes `ignoring_deprecations()`, like in the library. * An "error" filter, to check that a replacement API doesn't go through a deprecated one, becomes `asserting_no_deprecations()`. It records the warnings instead of raising them inside the code under test, so a broad `except` there can't swallow the failure, and it reports every offending warning with its location when the block ends. The blocks that record warnings to check what a helper lets through are left alone: that is what `catch_warnings(record=True)` is for. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

The previous commits moved the library and its tests to the `frequenz.core.warnings` helpers, but the guide still told contributors to silence inner deprecations with `warnings.catch_warnings()`, which is the exact bug they fix. Anyone following it would bring the warning amplification back. The deprecation guide gets two new sections and an updated one: * Keeping a moved symbol importable with `deprecated_aliases()`, including the `TYPE_CHECKING`/`else` structure it needs, a message carrying the version, and when a deprecated class is the better fit, as with `ComponentId`. * Silencing only the deprecations the library raises itself, with `ignoring_deprecations()` around the one statement, and why `catch_warnings()` must not be used for it. * Testing that a replacement doesn't warn with `asserting_no_deprecations()` instead of an "error" filter. The testing guide points at both helpers. The helpers are mentioned as plain code rather than cross-references because the frequenz-core inventory will only have them once v1.5.0 is released. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

llucax requested a review from a team as a code owner September 21, 2026 12:47

llucax requested review from Marenz and removed request for a team September 21, 2026 12:47

llucax self-assigned this Sep 21, 2026

llucax enabled auto-merge September 21, 2026 12:48

github-actions Bot added part:docs Affects the documentation part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) labels Sep 21, 2026

llucax disabled auto-merge September 21, 2026 12:54

llucax added the cmd:skip-release-notes It is not necessary to update release notes for this PR label Sep 21, 2026

llucax enabled auto-merge September 21, 2026 17:16

llucax requested a review from daniel-zullo-frequenz September 22, 2026 08:34

daniel-zullo-frequenz requested a balanced review from Copilot September 22, 2026 08:44

Copilot started reviewing on behalf of daniel-zullo-frequenz September 22, 2026 08:45 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

daniel-zullo-frequenz reviewed Sep 22, 2026

View reviewed changes

llucax force-pushed the deprecated-admonitions branch 2 times, most recently from 9e5cd51 to 441e8ef Compare September 22, 2026 13:53

llucax disabled auto-merge September 22, 2026 13:54

llucax marked this pull request as draft September 22, 2026 13:55

llucax added 3 commits September 28, 2026 15:43

llucax force-pushed the deprecated-admonitions branch from 441e8ef to f7e2a35 Compare September 28, 2026 16:24

llucax removed the cmd:skip-release-notes It is not necessary to update release notes for this PR label Sep 28, 2026

llucax changed the title ~~Style deprecation notices in the API reference~~ Adopt the frequenz-core deprecation tooling Sep 28, 2026

llucax force-pushed the deprecated-admonitions branch 2 times, most recently from 9409d1d to 2538fe8 Compare September 28, 2026 19:03

llucax added 4 commits September 28, 2026 20:53

llucax force-pushed the deprecated-admonitions branch from 2538fe8 to 46d6b2a Compare September 28, 2026 21:00

Navigation Menu

Adopt the frequenz-core deprecation tooling - #288

Adopt the frequenz-core deprecation tooling#288
llucax wants to merge 8 commits into
frequenz-floss:v0.x.xfrom
llucax:deprecated-admonitions

llucax commented Sep 21, 2026 •

edited

Loading

Uh oh!

llucax commented Sep 21, 2026

Uh oh!

llucax commented Sep 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniel-zullo-frequenz left a comment

Uh oh!

llucax commented Sep 22, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

llucax commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llucax commented Sep 21, 2026

Uh oh!

llucax commented Sep 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniel-zullo-frequenz left a comment

Choose a reason for hiding this comment

Uh oh!

llucax commented Sep 22, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

llucax commented Sep 21, 2026 •

edited

Loading

llucax commented Sep 21, 2026 •

edited

Loading