Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Several requirements conflict with mypy behavior, the existing semver policy, and the referenced client guide.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds a canonical Python deprecation policy and connects it to the existing 0.x.x versioning guidance.
Changes:
- Defines runtime, typing, documentation, testing, and release-note practices.
- Documents compatibility aliases for moved symbols.
- Links the versioning guide to the new policy.
| File | Description |
|---|---|
python/deprecations.md |
Adds the deprecation guide. |
python/semver-0.x.x.md |
Links to the guide. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
llucax
force-pushed
the
add-deprecations-doc
branch
from
September 22, 2026 07:32
9703ba9 to
04e28a6
Compare
Contributor
Author
|
This is a draft because it needs |
`semver-0.x.x.md` says when a deprecated symbol may be removed, but not how to deprecate one, and deprecation applies just as much to libraries past 1.0.0, so it can't live there. The document collects what was learned by actually doing this across our repositories: that a deprecation must never break downstream builds, that `typing_extensions.deprecated` covers the type checker, the rendered docs and the runtime warning from a single message, what PEP 702 explicitly left out and therefore what only the rendered admonition can reach, how to keep an old import path working through a module `__getattr__`, and the test and release note every deprecation needs. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Deprecation is what the 0.x.x rules lean on to keep patch releases backwards-compatible, so readers landing there need a way to find how it is actually done. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
llucax
force-pushed
the
add-deprecations-doc
branch
from
September 28, 2026 16:24
04e28a6 to
03ec4a8
Compare
llucax
force-pushed
the
add-deprecations-doc
branch
2 times, most recently
from
September 28, 2026 21:00
4e3863b to
cea05b6
Compare
The guide predates three things it now has to describe: the final `frequenz.core.warnings` API (frequenz-core#199), the `griffe-frequenz-core` extension, and the `Deprecated:` admonition convention from frequenz-repo-config#641. Enum members and moved symbols now have helpers that warn at runtime and, through `griffe_frequenz_core.deprecations`, get the same rendered admonition as a decorated symbol, so they move out of the "write it by hand" list, which keeps only function arguments, modules, constants and attributes. The alias example passes a `message` with the version, since the default template has none and the guide requires one. Library code touching its own deprecated symbols is told to use `ignoring_deprecations()` instead of `warnings.catch_warnings()`, which resets the deduplication history for the whole program, and tests get `asserting_no_deprecations()` to check that a replacement doesn't go through the deprecated symbol, a mistake the `once::DeprecationWarning` filter hides. The claim that hand-written admonitions go where the extension puts the generated ones was wrong: both extensions insert them above the summary, which a docstring can't do. The new last section shows the `mkdocs.yml` configuration for both extensions. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
The "Enum members" bullet says to use `frequenz.core.enum.Enum` but not how a member is actually marked, which the deprecations guide now covers with `deprecated_member()`, so link that section. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
The repository configuration template now adds a `filterwarnings` entry that turns deprecation warnings mentioning the project's own fully qualified names into errors, so a project can't release code still using symbols it deprecated itself. Dependencies' deprecations stay warnings, so this doesn't contradict the rule that a deprecation must never break downstream builds. The guide showed the configuration without it, and the message rule that makes it work was only loosely motivated, so both now point at each other, and the rule says the name must be plain text, since backticks or a cross-reference stop the filter from matching. The testing section assumed every deprecation was only reported once. It now says that tests using the project's own deprecated symbols on purpose need `pytest.deprecated_call()` or a `filterwarnings` mark, and that the filter catches a replacement still going through the deprecated symbol only heuristically, which is why `asserting_no_deprecations()` is still recommended. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
llucax
force-pushed
the
add-deprecations-doc
branch
from
September 30, 2026 13:27
7fb88d6 to
6956ff2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
python/deprecations.md, the canonical description of how we deprecate things in Python projects, and links it frompython/semver-0.x.x.md.It goes in its own document rather than into the semver one because deprecation applies just as much to libraries past 1.0.0, so the 0.x.x rules can't be its home. There is no 1.0-and-later versioning document in this repository yet; when there is one, it should link here too.
The content is what we learned doing this across our repositories, not a restatement of PEP 702. The parts most worth reviewing are the claim that a deprecation must never break downstream builds (which rules out
-W error::DeprecationWarningand hiding symbols from type checkers), and the section on what type checkers can never report, since aliases, re-exports, modules and constants keep being rediscovered as dead ends.It is deliberately consistent with frequenz-client-common's deprecation and compatibility guide, which stays as the client-specific policy sitting under this one.
It also covers the tooling that goes with it: the
frequenz.core.warningshelpers from frequenz-floss/frequenz-core-python#199, with the per-aliasDeprecatedAliasmessages used in thedeprecated_aliases()example from frequenz-floss/frequenz-core-python#200, both merged and released infrequenz-corev1.5.0; thegriffe-frequenz-coreextension that renders the deprecations those helpers declare; and theDeprecated:admonition convention from frequenz-floss/frequenz-repo-config-python#641.The
pytestconfiguration it shows includes afilterwarningsentry that turns the project's own deprecations into errors, matching messages that start with the deprecated symbol's fully qualified name, while deprecations from dependencies stay warnings. It comes from frequenz-floss/frequenz-repo-config-python#647, which adds it to the template and to the migration script. It is merged but not released yet.Draft until repo-config ships what the guide says the template provides: frequenz-floss/frequenz-repo-config-python#641 and frequenz-floss/frequenz-repo-config-python#647 are merged but not released yet, and the template support for the
griffe-frequenz-coreextension (including thegriffe-frequenz-corerelease that readsDeprecatedAlias) is still to come. Thefrequenz.core.warningslinks already resolve, asfrequenz-corev1.5.0 is released.