Skip to content

Make each deprecated alias take its own message - #200

Merged
llucax merged 3 commits into
frequenz-floss:v1.x.xfrom
llucax:deprecated-alias-messages
Oct 5, 2026
Merged

llucax merged 3 commits into
frequenz-floss:v1.x.xfrom
llucax:deprecated-alias-messages

Conversation

@llucax

@llucax llucax commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #199, as announced in #199 (comment): every alias built with deprecated_aliases() now says since which version it is deprecated, following the deprecations guideline.

__getattr__ = deprecated_aliases(
    __name__,
    DeprecatedAlias("Decimal", new_module="decimal", since="v1.2.0"),
    DeprecatedAlias(
        "Rational",
        new_module="fractions",
        new_name="Fraction",
        message="{old} is deprecated since v1.3.0 and will be removed in v2.0.0. Use {new} instead.",
    ),
    DeprecatedAlias("Gadget", new_name="Widget", since="v1.4.0"),  # Renamed in this module
)

Each entry says where the symbol is now with new_module, new_name or both, so the module:name string from #199 is gone, and a symbol renamed within the same module can be aliased too. It also gives either since, which warns with the guideline's wording (<old> is deprecated since <since>. Use <new> instead.), or a message template of its own. Invalid combinations raise a TypeError, and __init__ overloads make type checkers reject them too.

The mapping form and the table-wide message from #199 are gone rather than kept next to the new form: they were never released, and every alias should state its version. DeprecatedAlias validates itself when created, so a bad entry fails where it is written.

griffe-frequenz-core reads the new call shape to generate one Deprecated admonition per alias with its own message.

@github-actions github-actions Bot added part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) labels Sep 28, 2026
@llucax
llucax force-pushed the deprecated-alias-messages branch from 41afcdf to faca9a6 Compare September 28, 2026 20:46
llucax added a commit to llucax/griffe-frequenz-core that referenced this pull request Sep 28, 2026
The compatibility tests for `deprecated_aliases()` need a core that has
`frequenz.core.warnings` with `DeprecatedAlias`, which no release has
yet. This points the test-only pin at the head of
frequenz-floss/frequenz-core-python#200, through the upstream URL so the
version is built from upstream's tags as `1.4.0.postN`.

It must be replaced by `frequenz-core == 1.5.0` once that is released,
before merging.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
llucax force-pushed the deprecated-alias-messages branch 2 times, most recently from d75b5b0 to c12b4a0 Compare September 30, 2026 13:26
@llucax
llucax marked this pull request as ready for review September 30, 2026 13:27
@llucax
llucax requested a review from a team as a code owner September 30, 2026 13:27
@llucax
llucax requested review from shsms and removed request for a team September 30, 2026 13:27
@llucax llucax self-assigned this Sep 30, 2026
llucax added a commit to llucax/griffe-frequenz-core that referenced this pull request Sep 30, 2026
The compatibility tests for `deprecated_aliases()` need a core that has
`frequenz.core.warnings` with `DeprecatedAlias`, which no release has
yet. This points the test-only pin at the head of
frequenz-floss/frequenz-core-python#200, through the upstream URL so the
version is built from upstream's tags as `1.4.0.postN`.

It must be replaced by `frequenz-core == 1.5.0` once that is released,
before merging.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
llucax requested a review from Marenz September 30, 2026 13:27
@llucax llucax added this to the v1.5.0 milestone Sep 30, 2026
Marenz
Marenz previously approved these changes Oct 1, 2026

@Marenz Marenz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 The API and runtime behaviour look sound. One minor edge: message validation uses empty strings, so "{old:{new}}" passes construction but raises ValueError on lookup. "{old.missing}" also raises AttributeError rather than the documented ValueError. Non-blocking, but worth tightening.

@llucax
llucax force-pushed the deprecated-alias-messages branch from c12b4a0 to 698529e Compare October 1, 2026 16:34
llucax added a commit to llucax/griffe-frequenz-core that referenced this pull request Oct 1, 2026
The compatibility tests for `deprecated_aliases()` need a core that has
`frequenz.core.warnings` with `DeprecatedAlias`, which no release has
yet. This points the test-only pin at the head of
frequenz-floss/frequenz-core-python#200, through the upstream URL so the
version is built from upstream's tags as `1.4.0.postN`.

It must be replaced by `frequenz-core == 1.5.0` once that is released,
before merging.

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

llucax commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor Author

Fixed (diff).

@llucax
llucax enabled auto-merge October 1, 2026 16:36
@llucax
llucax requested review from Marenz and a balanced review from Copilot October 1, 2026 16:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The release notes omit the primary public API change, and one API docstring contains unclear grammar.

Review effort: Balanced
Findings: 2 Low severity

Open (2)
What changed in this PR

Introduces per-alias deprecation metadata and supports symbols renamed within their original module.

Changes:

  • Adds the immutable DeprecatedAlias API with validation.
  • Updates deprecated_aliases() to accept individual alias definitions.
  • Expands documentation and tests for messages, renames, and invalid inputs.
File Description
src/​frequenz/​core/​warnings/​_deprecated_aliases.py Implements the new alias model and resolution behavior.
src/​frequenz/​core/​warnings/​__init__.py Exports and documents DeprecatedAlias.
tests/​warnings/​test_deprecated_aliases.py Tests validation, messages, and alias resolution.
tests/​warnings/​documented_aliases/​__init__.py Updates the documented integration fixture.
README.md Revises the usage example.
RELEASE_NOTES.md Notes rename support.

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

Comment thread RELEASE_NOTES.md
Comment thread src/frequenz/core/warnings/_deprecated_aliases.py Outdated
@llucax llucax changed the title Give each deprecated alias its own message Make each deprecated alias take its own message Oct 5, 2026
`deprecated_aliases()` takes one message template for its whole table,
so a module whose aliases were deprecated in different releases can't
say in each warning which version deprecated it, which is what the
deprecations guide asks for (`<old> is deprecated since vX.Y.Z. Use
<new> instead.`).

`DeprecatedAlias` is added to wrap each entry of that table: the
deprecated name, where the symbol is now, and either the version it is
deprecated since or its own message. The next commit makes
`deprecated_aliases()` take these instead of a mapping.

Where the symbol is now is given as `new_module` and `new_name`, as two
arguments instead of one `module:name` string, so there is nothing to
parse and each part can be checked on its own. At least one of them is
required: without `new_name` the symbol kept its name, and without
`new_module` it was renamed in the module defining the alias, which a
single string could not say without repeating that module's name.

`since="v1.2.0"` is the common case and gives the guide's wording, so
most entries don't have to repeat it; `message` is a full template with
`{old}` and `{new}` for anything else. Exactly one of the two is
required. The next commit removes the table-wide default, so users
can't forget to include the version. `format_message()` builds the
warning from either, inserting `since` as written instead of splicing
it into a template, so a brace in it needs no escaping.

A `message` may only use plain `{old}` and `{new}` fields, with no
attribute, index, conversion or format spec, and that is checked by
parsing it. Formatting it once with placeholder names, as the
table-wide message was checked before, can't catch the rest: whether
`{old.missing}` or `{old:{new}}` fails depends on the real names, which
are only known when the alias is reached.

The name is positional-only and the rest keyword-only, so a call reads
as `DeprecatedAlias("Decimal", new_module="decimal", since="v1.2.0")`,
which says which string is which without having to know the order.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
llucax added 2 commits October 5, 2026 15:12
`deprecated_aliases()` now takes the module followed by one
`DeprecatedAlias` per alias, instead of a mapping and a single
`message` for all of them, so every alias says since which version it
is deprecated:

    __getattr__ = deprecated_aliases(
        __name__,
        DeprecatedAlias("Decimal", new_module="decimal", since="v1.2.0"),
    )

Each alias warns with its own `format_message()`: the deprecations
guide's wording for one with `since`, its own template for one with
`message`.

An alias without `new_module` is looked up in `module` itself, so a
symbol renamed in place keeps its old name too. An alias pointing at
another alias of the same module, or at itself, is a `ValueError`:
reaching it would call the same `__getattr__` again, and never stop if
the two point at each other.

Aliases passed as keyword arguments (`Decimal="decimal"`) were
considered and dropped: they would make `category` and `stacklevel`
names that can never be aliased.

Each entry already checked its own fields, so what is left to check is
about the set as a whole and fits in `deprecated_aliases()` itself,
and `_checked_aliases()` goes.

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
@llucax
llucax force-pushed the deprecated-alias-messages branch from 3474fe9 to ff53f4d Compare October 5, 2026 13:12

@Marenz Marenz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 The template-validation edge cases are fixed and covered by regression tests. Looks good.

@llucax
llucax added this pull request to the merge queue Oct 5, 2026
Merged via the queue into frequenz-floss:v1.x.x with commit 8a31d4a Oct 5, 2026
9 checks passed
@llucax
llucax deleted the deprecated-alias-messages branch October 5, 2026 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants