From 640f13ff60ab06ee2c5b0989b064f989570550a1 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 28 Sep 2026 17:46:09 +0000 Subject: [PATCH 1/3] Add DeprecatedAlias `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 (` is deprecated since vX.Y.Z. Use 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 --- src/frequenz/core/warnings/__init__.py | 3 +- .../core/warnings/_deprecated_aliases.py | 283 +++++++++++++++++- tests/warnings/test_deprecated_aliases.py | 182 +++++++++++ 3 files changed, 466 insertions(+), 2 deletions(-) diff --git a/src/frequenz/core/warnings/__init__.py b/src/frequenz/core/warnings/__init__.py index 6ad19a1..18494c4 100644 --- a/src/frequenz/core/warnings/__init__.py +++ b/src/frequenz/core/warnings/__init__.py @@ -56,10 +56,11 @@ def convert(value: str) -> int: """ from ._asserting import asserting_no_deprecations, asserting_no_warnings -from ._deprecated_aliases import deprecated_aliases +from ._deprecated_aliases import DeprecatedAlias, deprecated_aliases from ._ignoring import ignoring_deprecations, ignoring_warnings __all__ = [ + "DeprecatedAlias", "asserting_no_deprecations", "asserting_no_warnings", "deprecated_aliases", diff --git a/src/frequenz/core/warnings/_deprecated_aliases.py b/src/frequenz/core/warnings/_deprecated_aliases.py index 8881822..7e09816 100644 --- a/src/frequenz/core/warnings/_deprecated_aliases.py +++ b/src/frequenz/core/warnings/_deprecated_aliases.py @@ -7,10 +7,291 @@ """ import importlib +import string import warnings from collections.abc import Callable, Mapping +from dataclasses import dataclass from types import ModuleType -from typing import Any +from typing import Any, overload + + +@dataclass(frozen=True, init=False, slots=True) +class DeprecatedAlias: + """A symbol kept reachable from its old name. + + Each one is an entry for [`deprecated_aliases`][..deprecated_aliases], which + has the examples. + + The symbol moved to another module, `new_module`, was renamed, `new_name`, or + both. Without `new_module` it is still in the module defining the alias, under + its new name. + + An alias says either since which version it is deprecated, with `since`, or + its whole warning message, with `message`, but not both. `since="v1.2.0"` + warns with ` is deprecated since v1.2.0. Use instead.`, which is + what the deprecations guide suggests, so `message` is only needed to say + something else. + + Everything is checked when the alias is created rather than when it is + reached. + """ + + name: str + """The deprecated name, in the module defining the alias.""" + + new_module: str | None + """The fully qualified name of the module the symbol lives in now. + + `None` if it is still in the module defining the alias, under + [`new_name`][..new_name]. + """ + + new_name: str + """The name the symbol has now, [`name`][..name] if it was not renamed.""" + + since: str | None + """The version the alias is deprecated since, or `None` if it has a `message`. + + The warning says ` is deprecated since . Use instead.` + """ + + message: str | None + """The template for the warning message, or `None` if it has a `since`. + + It is formatted with `{old}` and `{new}`, the fully qualified names of the + alias and of the symbol it resolves to, as plain fields, without attributes, + indexes, conversions or format specs. + """ + + # One pair of overloads with `new_module` and one with only `new_name`, so a call + # with neither matches none of them. + @overload + def __init__( # noqa: D107 + self, + name: str, + /, + *, + new_module: str, + new_name: str | None = None, + since: str, + ) -> None: ... + + @overload + def __init__( # noqa: D107 + self, + name: str, + /, + *, + new_module: str, + new_name: str | None = None, + message: str, + ) -> None: ... + + @overload + def __init__( # noqa: D107 + self, + name: str, + /, + *, + new_module: None = None, + new_name: str, + since: str, + ) -> None: ... + + @overload + def __init__( # noqa: D107 + self, + name: str, + /, + *, + new_module: None = None, + new_name: str, + message: str, + ) -> None: ... + + def __init__( # noqa: DOC502 + self, + name: str, + /, + *, + new_module: str | None = None, + new_name: str | None = None, + since: str | None = None, + message: str | None = None, + ) -> None: + """Initialize this instance. + + At least one of `new_module` and `new_name` must be given, and exactly one + of `since` and `message`. + + Args: + name: The deprecated name, in the module defining the alias. + new_module: The fully qualified name of the module the symbol lives in + now, if it moved out of the module defining the alias. + new_name: The name the symbol has now, if it was renamed. + since: The version the alias is deprecated since, such as `"v1.2.0"`. + The warning then says ` is deprecated since . Use + instead.` + message: The template for the whole warning message instead, formatted + with `{old}` and `{new}`, the fully qualified names of the alias + and of the symbol it resolves to, as plain fields, without + attributes, indexes, conversions or format specs. + + Raises: + TypeError: If an argument is not a string, if neither `new_module` nor + `new_name` is given, or if not exactly one of `since` and + `message` is. + ValueError: If `name`, `new_name` or a part of `new_module` is not an + identifier, if `since` is empty, or if `message` is not a template + with only plain `{old}` and `{new}` fields. + """ + # The class is frozen, which blocks plain assignment here too. + object.__setattr__(self, "name", name) + object.__setattr__(self, "new_module", new_module) + object.__setattr__(self, "new_name", name if new_name is None else new_name) + object.__setattr__(self, "since", since) + object.__setattr__(self, "message", message) + self._validate(new_name) + + def format_message(self, *, old: str, new: str) -> str: + """Format the warning message for this alias. + + Args: + old: The fully qualified name of the alias. + new: The fully qualified name of the symbol it resolves to. + + Returns: + ` is deprecated since . Use instead.` if the alias + has a `since`, or its `message` formatted with `old` and `new`. + """ + if self.message is None: + return f"{old} is deprecated since {self.since}. Use {new} instead." + return self.message.format(old=old, new=new) + + def _validate(self, new_name: str | None) -> None: + """Check the fields. + + Args: + new_name: The `new_name` given to `__init__()`, before it defaulted to + `name`, to tell whether it was given at all. + + Raises: + TypeError: If neither `new_module` nor `new_name` is given, or if not + exactly one of `since` and `message` is. + """ + if self.new_module is None and new_name is None: + raise TypeError( + f"the alias {self.name!r} needs new_module, new_name or both, " + "got neither" + ) + if (self.since is None) == (self.message is None): + raise TypeError( + f"the alias {self.name!r} needs exactly one of since and message, " + f"got {'both' if self.since is not None else 'neither'}" + ) + self._validate_name() + self._validate_new_module() + self._validate_new_name() + self._validate_since() + self._validate_message() + + def _validate_name(self) -> None: + """Check that `name` is an identifier. + + Raises: + TypeError: If it is not a string. + ValueError: If it is not an identifier. + """ + if not isinstance(self.name, str): + raise TypeError(f"alias names must be str, got {self.name!r}") + if not self.name.isidentifier(): + raise ValueError(f"alias names must be identifiers, got {self.name!r}") + + def _validate_new_module(self) -> None: + """Check that `new_module`, if given, is a module name. + + Raises: + TypeError: If it is not a string. + ValueError: If one of its dotted parts is not an identifier. + """ + if self.new_module is None: + return + if not isinstance(self.new_module, str): + raise TypeError( + f"the new_module of {self.name!r} must be a str, " + f"got {self.new_module!r}" + ) + if not all(part.isidentifier() for part in self.new_module.split(".")): + raise ValueError( + f"the new_module of {self.name!r} is not a module name: " + f"{self.new_module!r}" + ) + + def _validate_new_name(self) -> None: + """Check that `new_name` is an identifier. + + Raises: + TypeError: If it is not a string. + ValueError: If it is not an identifier. + """ + if not isinstance(self.new_name, str): + raise TypeError( + f"the new_name of {self.name!r} must be a str, got {self.new_name!r}" + ) + if not self.new_name.isidentifier(): + raise ValueError( + f"the new_name of {self.name!r} is not an identifier: " + f"{self.new_name!r}" + ) + + def _validate_since(self) -> None: + """Check that `since`, if given, is not empty. + + Raises: + TypeError: If it is not a string. + ValueError: If it is empty or blank. + """ + if self.since is None: + return + if not isinstance(self.since, str): + raise TypeError( + f"the since of {self.name!r} must be a str, got {self.since!r}" + ) + if not self.since.strip(): + raise ValueError(f"the since of {self.name!r} is empty") + + def _validate_message(self) -> None: + """Check that `message`, if given, is a template with plain `{old}` and `{new}`. + + Raises: + TypeError: If it is not a string. + ValueError: If it has a stray brace, or a replacement field other than a + plain `{old}` or `{new}`. + """ + if self.message is None: + return + if not isinstance(self.message, str): + raise TypeError( + f"the message of {self.name!r} must be a str, got {self.message!r}" + ) + problem = ( + f"the message of {self.name!r} must be a template with only plain " + f"{{old}} and {{new}} fields, got {self.message!r}" + ) + try: + # A stray brace raises here. + parsed = list(string.Formatter().parse(self.message)) + except ValueError as error: + raise ValueError(problem) from error + # Anything more than a plain field can fail depending on the names it is + # formatted with, which are only known when the alias is reached, so + # formatting it once here can't tell: `{old.missing}` raises + # `AttributeError`, and `{old:{new}}` makes the new name a format spec. + if not all( + field in (None, "old", "new") and not spec and conversion is None + for _, field, spec, conversion in parsed + ): + raise ValueError(problem) def _checked_aliases( diff --git a/tests/warnings/test_deprecated_aliases.py b/tests/warnings/test_deprecated_aliases.py index 297199a..1371748 100644 --- a/tests/warnings/test_deprecated_aliases.py +++ b/tests/warnings/test_deprecated_aliases.py @@ -3,6 +3,7 @@ """Tests for `deprecated_aliases()`.""" +import dataclasses import decimal import fractions from types import ModuleType @@ -11,12 +12,193 @@ import pytest from frequenz.core.warnings import ( + DeprecatedAlias, _deprecated_aliases, asserting_no_deprecations, deprecated_aliases, ) from tests.warnings import documented_aliases +_MESSAGE = "{old} is deprecated since v1.2.3. Use {new} instead." + + +def test_deprecated_alias_keeps_what_it_was_given() -> None: + """Test that an alias keeps its fields as given, and can't be changed.""" + alias = DeprecatedAlias( + "Rational", new_module="fractions", new_name="Fraction", message=_MESSAGE + ) + + assert alias.name == "Rational" + assert alias.new_module == "fractions" + assert alias.new_name == "Fraction" + assert alias.since is None + assert alias.message == _MESSAGE + assert alias == DeprecatedAlias( + "Rational", new_module="fractions", new_name="Fraction", message=_MESSAGE + ) + with pytest.raises(dataclasses.FrozenInstanceError): + alias.new_name = "" # type: ignore[misc] + + since = DeprecatedAlias("Decimal", new_module="decimal", since="v1.2.3") + assert since.since == "v1.2.3" + assert since.message is None + + +def test_deprecated_alias_defaults_the_target() -> None: + """Test that a missing `new_name` is `name`, and a missing `new_module` `None`.""" + moved = DeprecatedAlias("Decimal", new_module="decimal", since="v1.2.3") + assert moved.new_module == "decimal" + assert moved.new_name == "Decimal" + assert moved == DeprecatedAlias( + "Decimal", new_module="decimal", new_name="Decimal", since="v1.2.3" + ) + + renamed = DeprecatedAlias("OldName", new_name="NewName", since="v1.2.3") + assert renamed.new_module is None + assert renamed.new_name == "NewName" + + +@pytest.mark.parametrize( + "args, kwargs", + [ + (("Decimal", "decimal", _MESSAGE), {}), + ((), {"name": "Decimal", "new_module": "decimal", "message": _MESSAGE}), + (("Decimal", "decimal"), {"since": "v1.2.3"}), + ], + ids=["all-positional", "name-by-keyword", "new-module-positional"], +) +def test_deprecated_alias_signature( + args: tuple[Any, ...], kwargs: dict[str, Any] +) -> None: + """Test that the name is positional-only and the rest keyword-only.""" + with pytest.raises(TypeError): + DeprecatedAlias(*args, **kwargs) + + +def test_deprecated_alias_needs_exactly_one_of_since_and_message() -> None: + """Test that giving both `since` and `message`, or neither, is refused. + + The `type: ignore` comments are part of the test: mypy reports them as unused, + and fails, if the overloads ever accept these calls. + """ + with pytest.raises(TypeError, match="exactly one of since and message, got both"): + DeprecatedAlias( # type: ignore[call-overload] + "Decimal", new_module="decimal", since="v1.2.3", message=_MESSAGE + ) + with pytest.raises( + TypeError, match="exactly one of since and message, got neither" + ): + DeprecatedAlias("Decimal", new_module="decimal") # type: ignore[call-overload] + + +def test_deprecated_alias_needs_a_new_module_or_name() -> None: + """Test that an alias saying neither where nor under which name is refused. + + The `type: ignore` comments are part of the test: mypy reports them as unused, + and fails, if the overloads ever accept these calls. + """ + with pytest.raises(TypeError, match="needs new_module, new_name or both"): + DeprecatedAlias("Decimal", since="v1.2.3") # type: ignore[call-overload] + with pytest.raises(TypeError, match="needs new_module, new_name or both"): + DeprecatedAlias( # type: ignore[call-overload] + "Decimal", new_module=None, new_name=None, message=_MESSAGE + ) + + +@pytest.mark.parametrize( + "name, new_module, new_name, message, error", + [ + (0, "decimal", None, _MESSAGE, TypeError), + ("", "decimal", None, _MESSAGE, ValueError), + ("Dec imal", "decimal", None, _MESSAGE, ValueError), + ("Decimal", 0, None, _MESSAGE, TypeError), + ("Decimal", "", None, _MESSAGE, ValueError), + ("Decimal", "a..b", None, _MESSAGE, ValueError), + ("Decimal", "a.b-c", None, _MESSAGE, ValueError), + ("Decimal", "decimal", 0, _MESSAGE, TypeError), + ("Decimal", "decimal", "", _MESSAGE, ValueError), + ("Decimal", "decimal", "decimal.Decimal", _MESSAGE, ValueError), + ("Decimal", "decimal", None, 0, TypeError), + ("Decimal", "decimal", None, "{old} moved, see {'here': 1}", ValueError), + ("Decimal", "decimal", None, "{old} moved to {where}", ValueError), + ("Decimal", "decimal", None, "{} moved to {new}", ValueError), + ("Decimal", "decimal", None, "{0} moved to {new}", ValueError), + ("Decimal", "decimal", None, "{old.missing} moved to {new}", ValueError), + ("Decimal", "decimal", None, "{old[0]} moved to {new}", ValueError), + ("Decimal", "decimal", None, "{old!r} moved to {new}", ValueError), + ("Decimal", "decimal", None, "{old:>20} moved to {new}", ValueError), + ("Decimal", "decimal", None, "{old:{new}} moved", ValueError), + ], + ids=[ + "name-type", + "name-empty", + "name-not-identifier", + "new-module-type", + "new-module-empty", + "new-module-empty-part", + "new-module-not-identifier", + "new-name-type", + "new-name-empty", + "new-name-dotted", + "message-type", + "message-stray-brace", + "message-unknown-field", + "message-auto-numbered-field", + "message-numbered-field", + "message-attribute", + "message-index", + "message-conversion", + "message-format-spec", + "message-nested-field", + ], +) +def test_deprecated_alias_rejects_bad_arguments( + name: Any, new_module: Any, new_name: Any, message: Any, error: type[Exception] +) -> None: + """Test that an alias is checked when it is created. + + Left to the lookup, an alias like this fails wherever it happens to be reached, + with an error that says nothing about deprecated aliases: a `ModuleNotFoundError` + for a module name with a typo, say, or a `KeyError` from inside + `warnings.warn()` for a stray brace in the message. + """ + with pytest.raises(error): + DeprecatedAlias(name, new_module=new_module, new_name=new_name, message=message) + + +@pytest.mark.parametrize( + "since, error", + [(0, TypeError), ("", ValueError), (" ", ValueError)], + ids=["type", "empty", "blank"], +) +def test_deprecated_alias_rejects_a_bad_since( + since: Any, error: type[Exception] +) -> None: + """Test that `since` is checked when the alias is created, like the rest.""" + with pytest.raises(error): + DeprecatedAlias("Decimal", new_module="decimal", since=since) + + +def test_deprecated_alias_formats_its_message() -> None: + """Test the message of an alias with `since` and of one with `message`. + + `since` is inserted as written, braces included, rather than being read as part + of a template. + """ + names = {"old": "old_home.Decimal", "new": "decimal.Decimal"} + + since = DeprecatedAlias("Decimal", new_module="decimal", since="v{1}.2") + assert since.format_message(**names) == ( + "old_home.Decimal is deprecated since v{1}.2. Use decimal.Decimal instead." + ) + + message = DeprecatedAlias( + "Decimal", new_module="decimal", message="{old} is gone, use {new} ({{v2}})" + ) + assert message.format_message(**names) == ( + "old_home.Decimal is gone, use decimal.Decimal ({v2})" + ) + def test_deprecated_aliases_warns_and_returns_the_real_object() -> None: """Test that reaching an alias warns and yields the object from its new home.""" From ca4648626b45249c89646cbb858e6d4372d3c796 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Wed, 30 Sep 2026 12:14:54 +0000 Subject: [PATCH 2/3] Take DeprecatedAlias entries in deprecated_aliases() `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 --- README.md | 13 +- src/frequenz/core/warnings/__init__.py | 4 +- .../core/warnings/_deprecated_aliases.py | 223 +++++++++--------- tests/warnings/documented_aliases/__init__.py | 19 +- tests/warnings/test_deprecated_aliases.py | 186 ++++++++++----- 5 files changed, 255 insertions(+), 190 deletions(-) diff --git a/README.md b/README.md index 10679d1..eb84862 100644 --- a/README.md +++ b/README.md @@ -194,7 +194,7 @@ object so `isinstance` keeps working through both paths: ```python from typing import TYPE_CHECKING, TypeAlias -from frequenz.core.warnings import deprecated_aliases +from frequenz.core.warnings import DeprecatedAlias, deprecated_aliases if TYPE_CHECKING: # Type checkers can't see the runtime `__getattr__` in the `else` branch. @@ -209,10 +209,13 @@ else: # imports included. __getattr__ = deprecated_aliases( __name__, - { - "Decimal": "decimal", # decimal.Decimal - "Rational": "fractions:Fraction", # Renamed on the way out - }, + # Warns ".Decimal is deprecated since v1.2.0. Use + # decimal.Decimal instead." + DeprecatedAlias("Decimal", new_module="decimal", since="v1.2.0"), + # Renamed on the way out + DeprecatedAlias( + "Rational", new_module="fractions", new_name="Fraction", since="v1.3.0" + ), ) ``` diff --git a/src/frequenz/core/warnings/__init__.py b/src/frequenz/core/warnings/__init__.py index 18494c4..92e5606 100644 --- a/src/frequenz/core/warnings/__init__.py +++ b/src/frequenz/core/warnings/__init__.py @@ -10,7 +10,9 @@ having to touch a symbol it deprecated itself. It also provides [`deprecated_aliases`][.deprecated_aliases], to keep the old import -path of a symbol that moved to another module working, warning whoever uses it, and +path of a symbol that moved to another module, or was renamed, working, warning +whoever uses it since which version it is deprecated, as its +[`DeprecatedAlias`][.DeprecatedAlias] entry says, and [`asserting_no_warnings`][.asserting_no_warnings] with its [`asserting_no_deprecations`][.asserting_no_deprecations] shortcut, to check in a test that a piece of code doesn't warn. diff --git a/src/frequenz/core/warnings/_deprecated_aliases.py b/src/frequenz/core/warnings/_deprecated_aliases.py index 7e09816..a8c057f 100644 --- a/src/frequenz/core/warnings/_deprecated_aliases.py +++ b/src/frequenz/core/warnings/_deprecated_aliases.py @@ -9,7 +9,7 @@ import importlib import string import warnings -from collections.abc import Callable, Mapping +from collections.abc import Callable from dataclasses import dataclass from types import ModuleType from typing import Any, overload @@ -294,69 +294,25 @@ def _validate_message(self) -> None: raise ValueError(problem) -def _checked_aliases( - module: str, aliases: Mapping[str, str] -) -> dict[str, tuple[str, str]]: - """Check a set of deprecated aliases and resolve them. - - Everything here is checked when the aliases are declared rather than when one - of them is reached, which can be a release later and in somebody else's code, - where the failure no longer looks like a typo in an alias table. - - Args: - module: The fully qualified name of the module defining the aliases. - aliases: The mapping to check. - - Returns: - Each deprecated name mapped to the module its target lives in and the name - it has there, so a later change to `aliases` can't get past these - checks and the string doesn't have to be taken apart on every lookup. - - Raises: - TypeError: If `module` is not a string, or a name or target is not one. - ValueError: If a target is empty, carries more than one `:`, or has - nothing after it. - """ - if not isinstance(module, str): - raise TypeError(f"module must be a str, got {module!r}") - checked: dict[str, tuple[str, str]] = {} - for name, target in dict(aliases).items(): - if not isinstance(name, str): - raise TypeError(f"alias names must be str, got {name!r}") - if not isinstance(target, str): - raise TypeError(f"the target of {name!r} must be a str, got {target!r}") - target_module, renamed, target_name = target.partition(":") - # `partition()` stops at the first colon, so a second one would silently - # end up inside the name, and the warning would point at `a.b:c`. - if ":" in target_name: - raise ValueError( - f"the target of {name!r} has more than one ':': {target!r}" - ) - if not target_module: - raise ValueError(f"the target of {name!r} names no module: {target!r}") - if renamed and not target_name: - raise ValueError( - f"the target of {name!r} has nothing after the ':': {target!r}" - ) - checked[name] = (target_module, target_name or name) - return checked - - def deprecated_aliases( # noqa: DOC502 module: str, - aliases: Mapping[str, str], - *, - message: str = "{old} is deprecated. Use {new} instead.", + /, + *aliases: DeprecatedAlias, category: type[Warning] = DeprecationWarning, stacklevel: int = 2, ) -> Callable[[str], Any]: """Build a module `__getattr__` that warns about deprecated aliases. - Use this for symbols that moved to another module but should keep working from - their old import path, when a [`typing_extensions.deprecated`][] decorator is not - an option because the object is not yours to mark: decorating it would deprecate - it for everybody, including the users of its new home. The alias also stays the - very same object, so [`isinstance`][] keeps working through both paths. + Use this for symbols that moved to another module, or were renamed, but should + keep working from their old import path, when a [`typing_extensions.deprecated`][] + decorator is not an option because the object is not yours to mark: decorating + it would deprecate it for everybody, including the users of its new home or name. + The alias also stays the very same object, so [`isinstance`][] keeps working + through both paths. + + Each [`DeprecatedAlias`][..DeprecatedAlias] says when that alias was deprecated + or provides its own message, so aliases deprecated in different releases can + emit different warnings. Danger: Follow the usage example structure strictly. In particular never drop @@ -371,7 +327,7 @@ def deprecated_aliases( # noqa: DOC502 ```python from typing import TYPE_CHECKING, TypeAlias - from frequenz.core.warnings import deprecated_aliases + from frequenz.core.warnings import DeprecatedAlias, deprecated_aliases if TYPE_CHECKING: # Private import, only for type checkers @@ -379,20 +335,24 @@ def deprecated_aliases( # noqa: DOC502 Decimal: TypeAlias = _Decimal else: - __getattr__ = deprecated_aliases(__name__, {"Decimal": "decimal"}) + __getattr__ = deprecated_aliases( + __name__, + DeprecatedAlias("Decimal", new_module="decimal", since="v1.2.0"), + ) ``` - Reaching `Decimal` through this module now emits a `DeprecationWarning` - saying to use `decimal.Decimal` instead. + Reaching `Decimal` through this module, say `mypkg.Decimal`, now emits a + `DeprecationWarning` saying `mypkg.Decimal is deprecated since v1.2.0. Use + decimal.Decimal instead.` Example: Renaming - When the symbol was also renamed on the way out, the new name goes after a - colon, as in an entry point: + When the symbol was also renamed on the way out, `new_name` gives the name + it has now: ```python from typing import TYPE_CHECKING, TypeAlias - from frequenz.core.warnings import deprecated_aliases + from frequenz.core.warnings import DeprecatedAlias, deprecated_aliases if TYPE_CHECKING: from fractions import Fraction as _Fraction @@ -401,21 +361,45 @@ def deprecated_aliases( # noqa: DOC502 else: __getattr__ = deprecated_aliases( __name__, - { - "Rational": "fractions:Fraction", - }, + DeprecatedAlias( + "Rational", + new_module="fractions", + new_name="Fraction", + since="v1.3.0", + ), + ) + ``` + + Without `new_module`, the symbol was renamed in this very module: + + ```python + from typing import TYPE_CHECKING, TypeAlias + + from frequenz.core.warnings import DeprecatedAlias, deprecated_aliases + + + class Widget: + ... # Called Gadget before v1.4.0 + + + if TYPE_CHECKING: + Gadget: TypeAlias = Widget + else: + __getattr__ = deprecated_aliases( + __name__, + DeprecatedAlias("Gadget", new_name="Widget", since="v1.4.0"), ) ``` - Example: Custom deprecation message - The message is a template that gets the old and new fully qualified names, - so it can carry a version, a link, or anything else the default doesn't - say: + Example: Custom message + When the standard wording is not enough, an alias can give its whole + message instead of `since`, as a template that gets the fully qualified old + and new names as `{old}` and `{new}`: ```python from typing import TYPE_CHECKING, TypeAlias - from frequenz.core.warnings import deprecated_aliases + from frequenz.core.warnings import DeprecatedAlias, deprecated_aliases if TYPE_CHECKING: from decimal import Decimal as _Decimal @@ -424,10 +408,12 @@ def deprecated_aliases( # noqa: DOC502 else: __getattr__ = deprecated_aliases( __name__, - { - "Decimal": "decimal", - }, - message="{old} is deprecated since v2, use {new} instead.", + DeprecatedAlias( + "Decimal", + new_module="decimal", + message="{old} is deprecated since v1.2.0 and will be removed " + "in v2.0.0. Use {new} instead.", + ), ) ``` @@ -438,19 +424,14 @@ def deprecated_aliases( # noqa: DOC502 that used to be reachable there. Warning: Security Warning - This function imports the target module, so the mapping is as trusted - as an `import` statement in this module. Write it out as a literal; - don't build it from anything that comes from outside the program. + This function imports the modules the aliases point at, so the aliases are + as trusted as an `import` statement in this module. Write them out as + literals; don't build them from anything that comes from outside the + program. Args: module: The fully qualified name of the module defining the aliases. - aliases: A mapping of each deprecated symbol to where it lives now, as the - fully qualified name of the module that owns it, optionally followed by - `:` and the name it has there, when it is not the deprecated one. It is - copied, so changing it afterwards has no effect. - message: The template for the warning message, formatted with `{old}` and - `{new}`, the fully qualified names of the alias and of the symbol it - resolves to. + *aliases: The deprecated aliases, at least one, each with its own name. category: The category of the warning to emit. stacklevel: How far up the stack the warning is reported, counting from the `__getattr__` itself. The default of 2 points at the code reaching for @@ -461,31 +442,41 @@ def deprecated_aliases( # noqa: DOC502 A function suitable for use as the module's `__getattr__`. Raises: - TypeError: If an argument has the wrong type, including a name or target - that is not a string. Also later, when an alias is reached, if it - resolves to a module rather than to a symbol in one. - ValueError: If a target is empty, carries more than one `:`, or has - nothing after it, if `message` is not a template taking `{old}` and - `{new}`, or if `stacklevel` is smaller than 1. + TypeError: If an argument has the wrong type, including an alias that is + not a [`DeprecatedAlias`][..DeprecatedAlias]. Also later, when an alias + is reached, if it resolves to a module rather than to a symbol in one. + ValueError: If there are no aliases, if two of them have the same name, if + one points at another alias in the same module, itself included, or if + `stacklevel` is smaller than 1. """ - if not isinstance(message, str): - raise TypeError(f"message must be a str, got {message!r}") + if not isinstance(module, str): + raise TypeError(f"module must be a str, got {module!r}") if not (isinstance(category, type) and issubclass(category, Warning)): raise TypeError(f"category must be a Warning subclass, got {category!r}") if not isinstance(stacklevel, int): raise TypeError(f"stacklevel must be an int, got {stacklevel!r}") if stacklevel < 1: raise ValueError(f"stacklevel must be 1 or more, got {stacklevel!r}") - try: - # Formatting it once here is the only way to find a stray brace, which - # would otherwise raise from inside `warnings.warn()` at lookup time. - message.format(old="", new="") - except (IndexError, KeyError, ValueError) as error: - raise ValueError( - f"message must be a template taking {{old}} and {{new}}, " - f"got {message!r}" - ) from error - targets = _checked_aliases(module, aliases) + if not aliases: + raise ValueError(f"no aliases given for module {module!r}") + by_name: dict[str, DeprecatedAlias] = {} + for alias in aliases: + # Also what a mapping, the form this function used to take, runs into. + if not isinstance(alias, DeprecatedAlias): + raise TypeError(f"aliases must be DeprecatedAlias instances, got {alias!r}") + # A dict literal would keep the last of two entries silently; this is the + # one place that can tell. + if alias.name in by_name: + raise ValueError(f"alias {alias.name!r} is given more than once") + by_name[alias.name] = alias + for alias in aliases: + # Reaching it would call this `__getattr__` again, and never stop if the + # aliases point at each other. + if alias.new_module in (None, module) and alias.new_name in by_name: + raise ValueError( + f"the alias {alias.name!r} points at {module}.{alias.new_name}, " + "which is a deprecated alias too" + ) def module_getattr(name: str) -> Any: """Return a deprecated alias, warning about its new location. @@ -501,28 +492,26 @@ def module_getattr(name: str) -> Any: TypeError: If the alias resolves to a module, which this can't keep reachable from its old path. """ - target = targets.get(name) - if target is None: + alias = by_name.get(name) + if alias is None: raise AttributeError(f"module {module!r} has no attribute {name!r}") - target_module, target_name = target + new_module = module if alias.new_module is None else alias.new_module + new = f"{new_module}.{alias.new_name}" # Resolved before the warning is raised, so an alias that can't work says # so instead of warning about a move that didn't happen. It also keeps the # `TypeError` below from being masked by an "error" filter turning that # warning into an exception first. - value = getattr(importlib.import_module(target_module), target_name) + value = getattr(importlib.import_module(new_module), alias.new_name) if isinstance(value, ModuleType): raise TypeError( - f"the target of {module}.{name} is the module " - f"{target_module}.{target_name}, and this aliases names, not " - "modules: the import system never consults a package's " - f"__getattr__, so `import {module}.{name}` would fail anyway. " - f"Keep a real __init__.py at {module}.{name}, with the aliases " - "for the names it used to hold in it." + f"the target of {module}.{name} is the module {new}, and this " + "aliases names, not modules: the import system never consults a " + f"package's __getattr__, so `import {module}.{name}` would fail " + f"anyway. Keep a real __init__.py at {module}.{name}, with the " + "aliases for the names it used to hold in it." ) warnings.warn( - message.format( - old=f"{module}.{name}", new=f"{target_module}.{target_name}" - ), + alias.format_message(old=f"{module}.{name}", new=new), category, stacklevel=stacklevel, ) diff --git a/tests/warnings/documented_aliases/__init__.py b/tests/warnings/documented_aliases/__init__.py index 4b751f0..5bd4279 100644 --- a/tests/warnings/documented_aliases/__init__.py +++ b/tests/warnings/documented_aliases/__init__.py @@ -12,7 +12,7 @@ from typing import TYPE_CHECKING, TypeAlias -from frequenz.core.warnings import deprecated_aliases +from frequenz.core.warnings import DeprecatedAlias, deprecated_aliases __all__ = ["Decimal", "Rational", "kept"] @@ -26,20 +26,27 @@ """A decimal number. Deprecated: - `tests.warnings.documented_aliases.Decimal` is deprecated. Use - [decimal.Decimal][] instead. + `tests.warnings.documented_aliases.Decimal` is deprecated since v1.2.0. + Use [decimal.Decimal][] instead. """ Rational: TypeAlias = _Fraction """A rational number. Deprecated: - `tests.warnings.documented_aliases.Rational` is deprecated. Use - [fractions.Fraction][] instead. + `tests.warnings.documented_aliases.Rational` is deprecated since v1.3.0. + Use [fractions.Fraction][] instead. """ else: __getattr__ = deprecated_aliases( - __name__, {"Decimal": "decimal", "Rational": "fractions:Fraction"} + __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. Use {new} instead.", + ), ) diff --git a/tests/warnings/test_deprecated_aliases.py b/tests/warnings/test_deprecated_aliases.py index 1371748..f515818 100644 --- a/tests/warnings/test_deprecated_aliases.py +++ b/tests/warnings/test_deprecated_aliases.py @@ -6,6 +6,7 @@ import dataclasses import decimal import fractions +import sys from types import ModuleType from typing import Any @@ -200,11 +201,24 @@ def test_deprecated_alias_formats_its_message() -> None: ) +_DECIMAL = DeprecatedAlias("Decimal", new_module="decimal", since="v1.2.3") + + def test_deprecated_aliases_warns_and_returns_the_real_object() -> None: - """Test that reaching an alias warns and yields the object from its new home.""" + """Test that reaching an alias warns with its own message and yields the object. + + `Decimal` gives `since`, and its expected warning spells out the standard + wording; `Fraction` gives a message of its own. + """ module = ModuleType("old_home") module.__getattr__ = deprecated_aliases( # type: ignore[method-assign] - module.__name__, {"Decimal": "decimal", "Fraction": "fractions"} + module.__name__, + _DECIMAL, + DeprecatedAlias( + "Fraction", + new_module="fractions", + message="{old} is gone since v2, see {new}", + ), ) with pytest.warns(DeprecationWarning) as caught: @@ -212,8 +226,8 @@ def test_deprecated_aliases_warns_and_returns_the_real_object() -> None: assert module.Fraction is fractions.Fraction assert [str(warning.message) for warning in caught] == [ - "old_home.Decimal is deprecated. Use decimal.Decimal instead.", - "old_home.Fraction is deprecated. Use fractions.Fraction instead.", + "old_home.Decimal is deprecated since v1.2.3. Use decimal.Decimal instead.", + "old_home.Fraction is gone since v2, see fractions.Fraction", ] # stacklevel=2, so the warning is attributed to this file, not to the helper. assert all(warning.filename == __file__ for warning in caught) @@ -223,7 +237,7 @@ def test_deprecated_aliases_rejects_unknown_names() -> None: """Test that a name that is not an alias raises as a missing attribute would.""" module = ModuleType("old_home_missing") module.__getattr__ = deprecated_aliases( # type: ignore[method-assign] - module.__name__, {"Decimal": "decimal"} + module.__name__, _DECIMAL ) with pytest.raises( @@ -233,82 +247,140 @@ def test_deprecated_aliases_rejects_unknown_names() -> None: @pytest.mark.parametrize( - "args, error", + "args, kwargs, error", [ - ((0, {"Decimal": "decimal"}), TypeError), - (("old_home_bad", {0: "decimal"}), TypeError), - (("old_home_bad", {"Decimal": 0}), TypeError), - (("old_home_bad", {"Decimal": ""}), ValueError), - (("old_home_bad", {"Decimal": ":Decimal"}), ValueError), - (("old_home_bad", {"Decimal": "decimal:"}), ValueError), - (("old_home_bad", {"Decimal": "a:b:c"}), ValueError), + ((0, _DECIMAL), {}, TypeError), + ((_DECIMAL,), {"module": "old_home_bad"}, TypeError), + (("old_home_bad",), {}, ValueError), + (("old_home_bad", {"Decimal": "decimal"}), {}, TypeError), + (("old_home_bad", "Decimal"), {}, TypeError), + ( + ( + "old_home_bad", + _DECIMAL, + DeprecatedAlias("Decimal", new_module="fractions", message=_MESSAGE), + ), + {}, + ValueError, + ), + ( + ( + "old_home_bad", + DeprecatedAlias("Decimal", new_name="Decimal", since="v1.2.3"), + ), + {}, + ValueError, + ), + ( + ( + "old_home_bad", + _DECIMAL, + DeprecatedAlias("Dec", new_name="Decimal", since="v1.2.3"), + ), + {}, + ValueError, + ), + ( + ( + "old_home_bad", + _DECIMAL, + DeprecatedAlias( + "Dec", new_module="old_home_bad", new_name="Decimal", since="v1.2.3" + ), + ), + {}, + ValueError, + ), ], ids=[ "module", - "name", - "target-type", - "target-empty", - "target-no-module", - "target-no-name", - "target-two-colons", + "module-by-keyword", + "no-aliases", + "mapping", + "not-an-alias", + "duplicate-name", + "points-at-itself", + "points-at-an-alias", + "points-at-an-alias-by-module", ], ) def test_deprecated_aliases_rejects_a_bad_table( - args: tuple[Any, Any], error: type[Exception] + args: tuple[Any, ...], kwargs: dict[str, Any], error: type[Exception] ) -> None: - """Test that the alias table is checked when it is declared. + """Test that the aliases are checked as a whole when they are declared. - Left to the lookup, a table like this fails wherever the alias happens to be - reached, with an error that says nothing about deprecated aliases: an - `AttributeError` about `str.partition` for a target that is not a string, say. + Each alias checks its own fields when it is created; what's left is the module, + the entries not being aliases at all, two aliases with the same name, where + one would silently win over the other, and an alias pointing at another one in + the same module, which would call the `__getattr__` again, forever if they + point at each other. """ with pytest.raises(error): - deprecated_aliases(*args) - - -def test_deprecated_aliases_copies_the_table() -> None: - """Test that changing the mapping afterwards doesn't get past the checks.""" - aliases: dict[str, str] = {"Decimal": "decimal"} - module = ModuleType("old_home_copied") - module.__getattr__ = deprecated_aliases( # type: ignore[method-assign] - module.__name__, aliases - ) - aliases["Fraction"] = "" - - with pytest.raises(AttributeError): - _ = module.Fraction + deprecated_aliases(*args, **kwargs) def test_deprecated_aliases_follows_renames() -> None: """Test that an alias can point to a symbol with a different name.""" module = ModuleType("old_home_renamed") module.__getattr__ = deprecated_aliases( # type: ignore[method-assign] - module.__name__, {"Rational": "fractions:Fraction"} + module.__name__, + DeprecatedAlias( + "Rational", new_module="fractions", new_name="Fraction", message=_MESSAGE + ), ) with pytest.warns( DeprecationWarning, match=( - "^old_home_renamed.Rational is deprecated. " + "^old_home_renamed.Rational is deprecated since v1.2.3. " "Use fractions.Fraction instead.$" ), ): assert module.Rational is fractions.Fraction +def test_deprecated_aliases_follows_renames_in_place( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Test that an alias without `new_module` points into its own module. + + The module is looked up through the import system like any other, so it has to + be in `sys.modules`, as a real one always is by the time it is reached. + """ + module = ModuleType("old_home_in_place") + module.Widget = fractions.Fraction # type: ignore[attr-defined] + module.__getattr__ = deprecated_aliases( # type: ignore[method-assign] + module.__name__, + DeprecatedAlias("Gadget", new_name="Widget", since="v1.2.3"), + ) + monkeypatch.setitem(sys.modules, module.__name__, module) + + with pytest.warns( + DeprecationWarning, + match=( + "^old_home_in_place.Gadget is deprecated since v1.2.3. " + "Use old_home_in_place.Widget instead.$" + ), + ): + assert module.Gadget is fractions.Fraction + + def test_deprecated_aliases_customises_the_warning() -> None: - """Test the message, category and stacklevel overrides.""" + """Test the category and stacklevel overrides.""" module = ModuleType("old_home_custom") module.__getattr__ = deprecated_aliases( # type: ignore[method-assign] module.__name__, - {"Decimal": "decimal"}, - message="{old} moved to {new} in v2", + _DECIMAL, category=FutureWarning, stacklevel=1, ) with pytest.warns( - FutureWarning, match="^old_home_custom.Decimal moved to decimal.Decimal in v2$" + FutureWarning, + match=( + "^old_home_custom.Decimal is deprecated since v1.2.3. " + "Use decimal.Decimal instead.$" + ), ) as caught: assert module.Decimal is decimal.Decimal @@ -319,17 +391,11 @@ def test_deprecated_aliases_customises_the_warning() -> None: @pytest.mark.parametrize( "kwargs, error", [ - ({"message": None}, TypeError), - ({"message": "{old} moved, see {'here': 1}"}, ValueError), - ({"message": "{old} moved to {where}"}, ValueError), ({"category": int}, TypeError), ({"stacklevel": "2"}, TypeError), ({"stacklevel": 0}, ValueError), ], ids=[ - "message-type", - "message-stray-brace", - "message-unknown-field", "category", "stacklevel-type", "stacklevel-range", @@ -338,13 +404,9 @@ def test_deprecated_aliases_customises_the_warning() -> None: def test_deprecated_aliases_rejects_bad_customisation( kwargs: dict[str, Any], error: type[Exception] ) -> None: - """Test that the overrides are checked when the aliases are declared. - - A template is checked by formatting it once here, since a stray brace would - otherwise raise a `KeyError` from inside `warnings.warn()`, at the lookup. - """ + """Test that the overrides are checked when the aliases are declared.""" with pytest.raises(error): - deprecated_aliases("old_home_bad", {"Decimal": "decimal"}, **kwargs) + deprecated_aliases("old_home_bad", _DECIMAL, **kwargs) def test_deprecated_aliases_in_a_real_module() -> None: @@ -359,7 +421,7 @@ def test_deprecated_aliases_in_a_real_module() -> None: with pytest.warns( DeprecationWarning, match=( - "^tests.warnings.documented_aliases.Decimal is deprecated. " + "^tests.warnings.documented_aliases.Decimal is deprecated since v1.2.0. " "Use decimal.Decimal instead.$" ), ): @@ -368,7 +430,7 @@ def test_deprecated_aliases_in_a_real_module() -> None: with pytest.warns( DeprecationWarning, match=( - "^tests.warnings.documented_aliases.Rational is deprecated. " + "^tests.warnings.documented_aliases.Rational is deprecated since v1.3.0. " "Use fractions.Fraction instead.$" ), ): @@ -412,9 +474,9 @@ def test_deprecated_aliases_reach_star_imports() -> None: # import itself, so every alias warns more than once here. Which messages come # out is the part worth asserting; how many times CPython looks them up is not. assert {str(warning.message) for warning in caught} == { - "tests.warnings.documented_aliases.Decimal is deprecated. " + "tests.warnings.documented_aliases.Decimal is deprecated since v1.2.0. " "Use decimal.Decimal instead.", - "tests.warnings.documented_aliases.Rational is deprecated. " + "tests.warnings.documented_aliases.Rational is deprecated since v1.3.0. " "Use fractions.Fraction instead.", } @@ -430,7 +492,9 @@ def test_deprecated_aliases_refuses_a_module_target() -> None: """ module = ModuleType("old_home_package") module.__getattr__ = deprecated_aliases( # type: ignore[method-assign] - module.__name__, {"path": "os"} # os.path is a module + module.__name__, + # os.path is a module + DeprecatedAlias("path", new_module="os", message=_MESSAGE), ) with pytest.raises(TypeError, match="^the target of old_home_package.path is"): From ff53f4dfbc90d18f385a03d650ea1b8603ed0cb4 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 28 Sep 2026 17:48:54 +0000 Subject: [PATCH 3/3] Update release notes Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 23775bd..c5fbc36 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -8,4 +8,4 @@ A new `frequenz.core.warnings` module with: - `asserting_no_warnings()`, and its `asserting_no_deprecations()` shortcut, fail when the code in the block raises a matching warning/deprecation, listing each one with the place it came from. They are meant for tests, and are a better tool than an `"error"` filter, which makes `warnings.warn()` raise inside the code under test and so changes the very behaviour the test is checking. -- `deprecated_aliases()` builds a module `__getattr__` that warns when a symbol that moved to another module is reached through its old import path, serving the very same object so `isinstance` keeps working through both paths. +- `deprecated_aliases()` builds a module `__getattr__` that warns when a symbol that moved to another module, or was renamed, is reached through its old import path, serving the very same object so `isinstance` keeps working through both paths.