| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
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>
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>
There was a problem hiding this comment.
🤖 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.
Sorry, something went wrong.
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>
|
Fixed (diff). |
Sorry, something went wrong.
There was a problem hiding this comment.
The release notes omit the primary public API change, and one API docstring contains unclear grammar.
Review effort: Balanced
Findings: 2
Introduces per-alias deprecation metadata and supports symbols renamed within their original module.
Changes:
| 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.
Sorry, something went wrong.
`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>
`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>
There was a problem hiding this comment.
🤖 The template-validation edge cases are fixed and covered by regression tests. Looks good.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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.
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.