From e7058a54ca6467fd253e3f1d28731a1c0733ed7d Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 23 Oct 2026 09:30:44 +0000 Subject: [PATCH 1/5] Separate the alias warning from its documentation Every deprecation has two texts for two different readers: the runtime warning, read in a terminal far from the deprecated symbol, and the `Deprecated:` notice in the API documentation, read right where it is documented. The deprecations guide (frequenz-floss/docs#6) now gives them different rules: the warning is plain text without the version, and the notice says since which version the symbol is deprecated and links its replacement. `DeprecatedAlias` mixed the two: `since` went into the warning, and `since` and `message` excluded each other, so an alias with a custom warning had no version left for `griffe-frequenz-core` to put in its notice. Now the standard warning is ` is deprecated. Use instead.`, `since` is only documentation metadata, and `message` only replaces the warning, so an alias can give both, either or neither. At least one of `new_module` and `new_name` is still required, which is all the overloads have to check now, so the four collapse into two. Only the text of the standard warning changes, and the check that is dropped only refused calls that are now valid, so nothing that worked before breaks. The docs now say what `since` and `message` are for, and that a hand-written `Deprecated:` admonition on the alias declared for type checkers is how the notice is changed. Signed-off-by: Leandro Lucarella --- README.md | 4 +- src/frequenz/core/warnings/__init__.py | 5 +- .../core/warnings/_deprecated_aliases.py | 120 ++++++++---------- tests/warnings/documented_aliases/__init__.py | 15 +-- tests/warnings/test_deprecated_aliases.py | 73 ++++++----- 5 files changed, 106 insertions(+), 111 deletions(-) diff --git a/README.md b/README.md index eb84862..613c64b 100644 --- a/README.md +++ b/README.md @@ -209,8 +209,8 @@ else: # imports included. __getattr__ = deprecated_aliases( __name__, - # Warns ".Decimal is deprecated since v1.2.0. Use - # decimal.Decimal instead." + # Warns ".Decimal is deprecated. Use decimal.Decimal instead.", + # `since` is for the documentation. DeprecatedAlias("Decimal", new_module="decimal", since="v1.2.0"), # Renamed on the way out DeprecatedAlias( diff --git a/src/frequenz/core/warnings/__init__.py b/src/frequenz/core/warnings/__init__.py index f00103a..e857750 100644 --- a/src/frequenz/core/warnings/__init__.py +++ b/src/frequenz/core/warnings/__init__.py @@ -13,8 +13,9 @@ It also provides [`deprecated_aliases`][frequenz.core.warnings.deprecated_aliases], to keep the old import 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`][frequenz.core.warnings.DeprecatedAlias] entry says, and +warning whoever uses it, with a +[`DeprecatedAlias`][frequenz.core.warnings.DeprecatedAlias] entry saying where it +is now and since which version it is deprecated, and [`asserting_no_warnings`][frequenz.core.warnings.asserting_no_warnings] with its [`asserting_no_deprecations`][frequenz.core.warnings.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 15f4f33..80da92b 100644 --- a/src/frequenz/core/warnings/_deprecated_aliases.py +++ b/src/frequenz/core/warnings/_deprecated_aliases.py @@ -27,11 +27,17 @@ class DeprecatedAlias: 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. + Reaching the alias warns with ` is deprecated. Use instead.`, with + fully qualified names, following the Frequenz deprecation guide, or with + `message` if something else is needed. + + `since`, the version the alias is deprecated since, is not part of the + warning: it is only there for the documentation. + [`griffe-frequenz-core`](https://github.com/frequenz-floss/griffe-frequenz-core) + reads it, together with where the symbol is now, to generate the alias' + `Deprecated:` notice, so `message` changes the warning only. To change the + notice, write a `Deprecated:` admonition by hand in the docstring of the alias + declared for type checkers, which replaces the generated one. Everything is checked when the alias is created rather than when it is reached. @@ -55,21 +61,21 @@ class DeprecatedAlias: """ since: str | None - """The version the alias is deprecated since, or `None` if it has a `message`. + """The version the alias is deprecated since, or `None` if not given. - The warning says ` is deprecated since . Use instead.` + It is only used for the documentation, never in the warning. """ message: str | None - """The template for the warning message, or `None` if it has a `since`. + """The template for the warning message, or `None` for the standard one. 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. + # One overload with `new_module` and one with only `new_name`, so a call with + # neither matches none of them. @overload def __init__( # noqa: D107 self, @@ -78,29 +84,8 @@ def __init__( # noqa: D107 *, 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, + since: str | None = None, + message: str | None = None, ) -> None: ... @overload @@ -111,7 +96,8 @@ def __init__( # noqa: D107 *, new_module: None = None, new_name: str, - message: str, + since: str | None = None, + message: str | None = None, ) -> None: ... def __init__( # noqa: DOC502 @@ -126,26 +112,24 @@ def __init__( # noqa: DOC502 ) -> None: """Initialize this instance. - At least one of `new_module` and `new_name` must be given, and exactly one - of `since` and `message`. + At least one of `new_module` and `new_name` must be given. 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. + since: The version the alias is deprecated since, such as `"v1.2.0"`, + for the documentation only. + message: The template for the warning message, replacing the standard + ` is deprecated. Use 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. + TypeError: If an argument is not a string, or if neither `new_module` + nor `new_name` is given. 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. @@ -166,11 +150,11 @@ def format_message(self, *, old: str, new: str) -> str: 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`. + The alias' `message` formatted with `old` and `new`, or + ` is deprecated. Use instead.` if it has none. """ if self.message is None: - return f"{old} is deprecated since {self.since}. Use {new} instead." + return f"{old} is deprecated. Use {new} instead." return self.message.format(old=old, new=new) def _validate(self, new_name: str | None) -> None: @@ -181,19 +165,13 @@ def _validate(self, new_name: str | None) -> None: `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. + TypeError: If neither `new_module` nor `new_name` is given. """ 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() @@ -315,9 +293,10 @@ def deprecated_aliases( # noqa: DOC502 The alias also stays the very same object, so [`isinstance`][] keeps working through both paths. - Each [`DeprecatedAlias`][frequenz.core.warnings.DeprecatedAlias] says when that - alias was deprecated or provides its own message, so aliases deprecated in - different releases can emit different warnings. + Each [`DeprecatedAlias`][frequenz.core.warnings.DeprecatedAlias] says where the + symbol is now and since which version the alias is deprecated, so aliases + deprecated in different releases are documented with their own version, and + can give their own warning message. Danger: Follow the usage example structure strictly. In particular never drop @@ -347,9 +326,14 @@ def deprecated_aliases( # noqa: DOC502 ``` Reaching `Decimal` through this module, say `mypkg.Decimal`, now emits a - `DeprecationWarning` saying `mypkg.Decimal is deprecated since v1.2.0. Use + `DeprecationWarning` saying `mypkg.Decimal is deprecated. Use decimal.Decimal instead.` + `since` doesn't change the warning: it is for + [`griffe-frequenz-core`](https://github.com/frequenz-floss/griffe-frequenz-core), + which documents the alias with a ``Deprecated since v1.2.0. Use + [`decimal.Decimal`][] instead.`` notice. + Example: Renaming When the symbol was also renamed on the way out, `new_name` gives the name it has now: @@ -397,9 +381,9 @@ class Widget: ``` 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}`: + When the standard warning is not enough, an alias can give its own + message, as a template that gets the fully qualified old and new names as + `{old}` and `{new}`: ```python from typing import TYPE_CHECKING, TypeAlias @@ -416,12 +400,18 @@ class Widget: DeprecatedAlias( "Decimal", new_module="decimal", - message="{old} is deprecated since v1.2.0 and will be removed " - "in v2.0.0. Use {new} instead.", + since="v1.2.0", + message="{old} is deprecated and will be removed in v2.0.0. " + "Use {new} instead.", ), ) ``` + The message replaces the warning only: the notice is still generated from + `since` and where the symbol is now. To say more in the documentation too, + write a `Deprecated:` admonition by hand in the docstring of `Decimal` in + the `if TYPE_CHECKING:` branch, which replaces the generated notice. + Tip: Deprecating whole modules This function can't be used to deprecate a whole module. If you need to do that, you can keep a real `__init__.py` at the old path, with a diff --git a/tests/warnings/documented_aliases/__init__.py b/tests/warnings/documented_aliases/__init__.py index 5bd4279..43354ec 100644 --- a/tests/warnings/documented_aliases/__init__.py +++ b/tests/warnings/documented_aliases/__init__.py @@ -23,19 +23,14 @@ from fractions import Fraction as _Fraction Decimal: TypeAlias = _Decimal - """A decimal number. - - Deprecated: - `tests.warnings.documented_aliases.Decimal` is deprecated since v1.2.0. - Use [decimal.Decimal][] instead. - """ + """A decimal number.""" Rational: TypeAlias = _Fraction """A rational number. Deprecated: - `tests.warnings.documented_aliases.Rational` is deprecated since v1.3.0. - Use [fractions.Fraction][] instead. + Deprecated since v1.3.0, and will be removed in v2.0.0. Use + [`fractions.Fraction`][] instead. """ else: __getattr__ = deprecated_aliases( @@ -45,7 +40,9 @@ "Rational", new_module="fractions", new_name="Fraction", - message="{old} is deprecated since v1.3.0. Use {new} instead.", + since="v1.3.0", + message="{old} is deprecated and will be removed in v2.0.0. " + "Use {new} instead.", ), ) diff --git a/tests/warnings/test_deprecated_aliases.py b/tests/warnings/test_deprecated_aliases.py index f515818..e6ee863 100644 --- a/tests/warnings/test_deprecated_aliases.py +++ b/tests/warnings/test_deprecated_aliases.py @@ -20,7 +20,7 @@ ) from tests.warnings import documented_aliases -_MESSAGE = "{old} is deprecated since v1.2.3. Use {new} instead." +_MESSAGE = "{old} is deprecated and will be removed. Use {new} instead." def test_deprecated_alias_keeps_what_it_was_given() -> None: @@ -76,20 +76,26 @@ def test_deprecated_alias_signature( 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. +def test_deprecated_alias_takes_since_and_message_independently() -> None: + """Test that `since` and `message` can both be given, or neither. - The `type: ignore` comments are part of the test: mypy reports them as unused, - and fails, if the overloads ever accept these calls. + `since` is only for the documentation and `message` only for the warning, so + neither excludes the other. These calls type check too: mypy fails on them if + the overloads ever stop accepting them. """ - 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] + both = DeprecatedAlias( + "Decimal", new_module="decimal", since="v1.2.3", message=_MESSAGE + ) + assert both.since == "v1.2.3" + assert both.message == _MESSAGE + + neither = DeprecatedAlias("Decimal", new_module="decimal") + assert neither.since is None + assert neither.message is None + + renamed = DeprecatedAlias("OldName", new_name="NewName") + assert renamed.since is None + assert renamed.message is None def test_deprecated_alias_needs_a_new_module_or_name() -> None: @@ -181,20 +187,24 @@ def test_deprecated_alias_rejects_a_bad_since( def test_deprecated_alias_formats_its_message() -> None: - """Test the message of an alias with `since` and of one with `message`. + """Test the standard message, and an alias' own `message`. - `since` is inserted as written, braces included, rather than being read as part - of a template. + `since` never shows up in either: it is only for the documentation. """ names = {"old": "old_home.Decimal", "new": "decimal.Decimal"} + standard = "old_home.Decimal is deprecated. Use decimal.Decimal instead." 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." - ) + assert since.format_message(**names) == standard + + neither = DeprecatedAlias("Decimal", new_module="decimal") + assert neither.format_message(**names) == standard message = DeprecatedAlias( - "Decimal", new_module="decimal", message="{old} is gone, use {new} ({{v2}})" + "Decimal", + new_module="decimal", + since="v1.2.3", + message="{old} is gone, use {new} ({{v2}})", ) assert message.format_message(**names) == ( "old_home.Decimal is gone, use decimal.Decimal ({v2})" @@ -226,7 +236,7 @@ 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 since v1.2.3. Use decimal.Decimal instead.", + "old_home.Decimal is deprecated. 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. @@ -332,7 +342,7 @@ def test_deprecated_aliases_follows_renames() -> None: with pytest.warns( DeprecationWarning, match=( - "^old_home_renamed.Rational is deprecated since v1.2.3. " + "^old_home_renamed.Rational is deprecated and will be removed. " "Use fractions.Fraction instead.$" ), ): @@ -358,7 +368,7 @@ def test_deprecated_aliases_follows_renames_in_place( with pytest.warns( DeprecationWarning, match=( - "^old_home_in_place.Gadget is deprecated since v1.2.3. " + "^old_home_in_place.Gadget is deprecated. " "Use old_home_in_place.Widget instead.$" ), ): @@ -377,10 +387,7 @@ def test_deprecated_aliases_customises_the_warning() -> None: with pytest.warns( FutureWarning, - match=( - "^old_home_custom.Decimal is deprecated since v1.2.3. " - "Use decimal.Decimal instead.$" - ), + match="^old_home_custom.Decimal is deprecated. Use decimal.Decimal instead.$", ) as caught: assert module.Decimal is decimal.Decimal @@ -421,7 +428,7 @@ def test_deprecated_aliases_in_a_real_module() -> None: with pytest.warns( DeprecationWarning, match=( - "^tests.warnings.documented_aliases.Decimal is deprecated since v1.2.0. " + "^tests.warnings.documented_aliases.Decimal is deprecated. " "Use decimal.Decimal instead.$" ), ): @@ -430,8 +437,8 @@ def test_deprecated_aliases_in_a_real_module() -> None: with pytest.warns( DeprecationWarning, match=( - "^tests.warnings.documented_aliases.Rational is deprecated since v1.3.0. " - "Use fractions.Fraction instead.$" + "^tests.warnings.documented_aliases.Rational is deprecated and will be " + "removed in v2.0.0. Use fractions.Fraction instead.$" ), ): assert documented_aliases.Rational is fractions.Fraction @@ -474,10 +481,10 @@ 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 since v1.2.0. " + "tests.warnings.documented_aliases.Decimal is deprecated. " "Use decimal.Decimal instead.", - "tests.warnings.documented_aliases.Rational is deprecated since v1.3.0. " - "Use fractions.Fraction instead.", + "tests.warnings.documented_aliases.Rational is deprecated and will be " + "removed in v2.0.0. Use fractions.Fraction instead.", } From dbb5c3b6ca55aa216b5a463a135b44750bda5f2e Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 23 Oct 2026 09:33:46 +0000 Subject: [PATCH 2/5] Let deprecated_member() take new_name and since A deprecated enum member only had its runtime message, so every one needed a hand-written `Deprecated:` notice repeating the same information, even for a plain rename. Client libraries can potentially deprecate (rename) dozens of members in a release. `deprecated_member()` and `DeprecatedMember` now take the same structured information as `DeprecatedAlias`: a keyword-only `new_name`, the member to use instead in the same enum, and `since`, the version it is deprecated since, which is only documentation metadata for `griffe-frequenz-core`. `message` gets a `None` default, and at least one of `message` and `new_name` is required, which overloads let mypy check. PENDING = deprecated_member(1, new_name="OPEN", since="v1.2.0") Without `message`, the warning is generated as `. is deprecated. Use . instead.`, with fully qualified names, as the deprecations guide asks. It can only be built when the enum class is created, since only then are its module and qualified name known, so the metaclass generates it then. It also checks that `new_name` is a member of the same enum and is not deprecated itself, so a typo or a chain of deprecations fails at import time instead of pointing users at something that doesn't work. `message` stays a literal message rather than an `{old}`/`{new}` template like `DeprecatedAlias`'s, since existing messages may contain braces, and it is still not checked, as before. Existing calls keep working, since `value` and `message` keep their position and names and the new parameters are keyword-only. The only visible change is that `DeprecatedMember.message` is now `str | None`, as a generated message is not known when the member is declared. The class is documented as not meant to be used directly, so this is acceptable in a minor release; storing the generated message back into it would have kept the type but left it without a meaningful value until the class is created. Signed-off-by: Leandro Lucarella --- src/frequenz/core/enum.py | 220 +++++++++++++++++++++++++++++++++++--- tests/test_enum.py | 156 +++++++++++++++++++++++++++ 2 files changed, 362 insertions(+), 14 deletions(-) diff --git a/src/frequenz/core/enum.py b/src/frequenz/core/enum.py index 4bede1b..61d6392 100644 --- a/src/frequenz/core/enum.py +++ b/src/frequenz/core/enum.py @@ -14,7 +14,7 @@ import enum import warnings from collections.abc import Mapping -from typing import TYPE_CHECKING, Any, ClassVar, Self, TypeVar, cast +from typing import TYPE_CHECKING, Any, ClassVar, Self, TypeVar, cast, overload # Note: This module contains more casts and uses of Any than what's typically # ideal. This is because type hinting EnumType and Enum subclasses is quite @@ -47,28 +47,215 @@ class DeprecatedMember: # Using slots is just an optimization to make the class more lightweight and avoid # the creation of a `__dict__` for each instance and its corresponding lookup. - __slots__ = ("value", "message") + __slots__ = ("value", "message", "new_name", "since") + + # One overload with `message` and one with only `new_name`, so a call with + # neither matches none of them. + @overload + def __init__( # noqa: D107 + self, + value: Any, + message: str, + *, + new_name: str | None = None, + since: str | None = None, + ) -> None: ... + + @overload + def __init__( # noqa: D107 + self, + value: Any, + message: None = None, + *, + new_name: str, + since: str | None = None, + ) -> None: ... + + def __init__( # noqa: DOC502 + self, + value: Any, + message: str | None = None, + *, + new_name: str | None = None, + since: str | None = None, + ) -> None: + """Initialize this instance. + + At least one of `message` and `new_name` must be given. + + Args: + value: The value of the enum member to mark as deprecated. + message: The warning message to emit when the member is reached, or + `None` to generate it from `new_name`. + new_name: The name of the member to use instead, in the same enum. + since: The version the member is deprecated since, such as `"v1.2.0"`, + for the documentation only. + + Raises: + TypeError: If neither `message` nor `new_name` is given, or if + `new_name` or `since` is not a string. + ValueError: If `new_name` is not an identifier, or if `since` is empty. + """ + self.value: Any = value + """The value of the deprecated member.""" + + self.message: str | None = message + """The warning message, or `None` if it is generated from `new_name`. + + A generated message is only known once the enum class is created, since it + includes the class' fully qualified name, and is not stored here. + """ - def __init__(self, value: Any, message: str) -> None: - """Initialize this instance.""" - self.value = value - self.message = message + self.new_name: str | None = new_name + """The name of the member to use instead, in the same enum, if given.""" + + self.since: str | None = since + """The version the member is deprecated since, if given. + + It is only used for the documentation, never in the warning. + """ + self._validate() -def deprecated_member(value: ValueT, message: str) -> ValueT: + def _validate(self) -> None: + """Check the fields. + + `message` is not checked, as it never was before `new_name` and `since` + were added, so existing code passing something else keeps working. + + Raises: + TypeError: If neither `message` nor `new_name` is given, or if + `new_name` or `since` is not a string. + ValueError: If `new_name` is not an identifier, or if `since` is empty. + """ + if self.message is None and self.new_name is None: + raise TypeError( + "a deprecated member needs message, new_name or both, got neither" + ) + if self.new_name is not None: + if not isinstance(self.new_name, str): + raise TypeError(f"new_name must be a str, got {self.new_name!r}") + if not self.new_name.isidentifier(): + raise ValueError( + f"new_name must be an identifier, got {self.new_name!r}" + ) + if self.since is not None: + if not isinstance(self.since, str): + raise TypeError(f"since must be a str, got {self.since!r}") + if not self.since.strip(): + raise ValueError("since must not be empty") + + +@overload +def deprecated_member( + value: ValueT, + message: str, + *, + new_name: str | None = None, + since: str | None = None, +) -> ValueT: ... + + +@overload +def deprecated_member( + value: ValueT, + message: None = None, + *, + new_name: str, + since: str | None = None, +) -> ValueT: ... + + +def deprecated_member( # noqa: DOC502 + value: ValueT, + message: str | None = None, + *, + new_name: str | None = None, + since: str | None = None, +) -> ValueT: """Mark an enum member as deprecated. + At least one of `message` and `new_name` must be given. With `new_name` alone, + reaching the member warns with `. is deprecated. Use + . instead.`, with fully qualified names, which is what the + deprecations guide asks for. `message` replaces that warning. + + `since` is not part of the warning: it is only there for the documentation. + [`griffe-frequenz-core`](https://github.com/frequenz-floss/griffe-frequenz-core) + reads it, together with `new_name`, to generate the member's `Deprecated:` + notice, so `message` changes the warning only. To change the notice, write a + `Deprecated:` admonition by hand in the member's docstring, which replaces the + generated one. + + When the enum class is created, `new_name` is checked to be another member of + the same enum, which is not deprecated itself. + Please read the [`Enum`][frequenz.core.enum.Enum] documentation for details and examples. Args: value: The value of the enum member to mark as deprecated. - message: The deprecation message to be shown when the member is accessed. + message: The warning message to emit when the member is reached, or `None` + to generate it from `new_name`. + new_name: The name of the member to use instead, in the same enum. + since: The version the member is deprecated since, such as `"v1.2.0"`, for + the documentation only. Returns: The wrapped value, to mark the enum member as deprecated. + + Raises: + TypeError: If neither `message` nor `new_name` is given, or if `new_name` + or `since` is not a string. + ValueError: If `new_name` is not an identifier, or if `since` is empty. + """ + # The overloads of `deprecated_member()` already checked the combination, which + # mypy can't tell from the `| None` types here. + member = DeprecatedMember( + value, message, new_name=new_name, since=since # type: ignore[arg-type] + ) + return cast(ValueT, member) + + +def _deprecation_messages( + cls: type[enum.Enum], deprecated: Mapping[str, DeprecatedMember] +) -> dict[str, str]: + """Return the warning message of each deprecated member of a new enum class. + + Args: + cls: The enum class just created. + deprecated: The deprecated members of `cls`, by name. + + Returns: + The warning message for each name in `deprecated`: its own `message`, or + one generated from its `new_name`. + + Raises: + ValueError: If a `new_name` is not a member of `cls`, or is deprecated + itself. """ - return cast(ValueT, DeprecatedMember(value, message)) + prefix = f"{cls.__module__}.{cls.__qualname__}" + messages: dict[str, str] = {} + for name, member in deprecated.items(): + if member.new_name is not None: + if member.new_name not in cls.__members__: + raise ValueError( + f"the new_name of {prefix}.{name} is not a member of " + f"{prefix}: {member.new_name!r}" + ) + if member.new_name in deprecated: + raise ValueError( + f"the new_name of {prefix}.{name} is {member.new_name!r}, " + "which is deprecated too" + ) + if member.message is not None: + messages[name] = member.message + else: + messages[name] = ( + f"{prefix}.{name} is deprecated. Use {prefix}.{member.new_name} " + "instead." + ) + return messages class DeprecatingEnumType(enum.EnumType): @@ -80,11 +267,15 @@ class DeprecatingEnumType(enum.EnumType): Behavior: - - In the class body, members may be declared as `NAME = DeprecatedMember(value, msg)`. - See [`DeprecatedMember`][frequenz.core.enum.DeprecatedMember] and - [`deprecated_member()`][frequenz.core.enum.deprecated_member] for details. + - In the class body, members may be declared as + `NAME = deprecated_member(value, ...)`. See + [`deprecated_member()`][frequenz.core.enum.deprecated_member] and + [`DeprecatedMember`][frequenz.core.enum.DeprecatedMember] for details. - During class creation, these wrappers are replaced with `value` so that a normal enum member or alias is created by [`EnumType`][enum.EnumType]. + - Once the class is created, each `new_name` is checked to be a member that is + not deprecated, and the warnings of members without a `message` are + generated from it, now that the fully qualified name of the class is known. - The deprecated names are recorded so that: * `MyEnum.NAME` warns (attribute access by name) @@ -106,18 +297,19 @@ def __new__( # pylint: disable=too-many-locals [`DeprecatedMember`][frequenz.core.enum.DeprecatedMember] are replaced with their value. """ - deprecated_names: dict[str, str] = {} + deprecated: dict[str, DeprecatedMember] = {} prepared = super().__prepare__(name, bases, **kw) # Unwrap DeprecatedMembers and record them as deprecated for key, value in classdict.items(): if isinstance(value, DeprecatedMember): - deprecated_names[key] = value.message + deprecated[key] = value prepared[key] = value.value else: prepared[key] = value cls = cast(type[EnumT], super().__new__(mcs, name, bases, prepared, **kw)) + deprecated_names = _deprecation_messages(cls, deprecated) # Build alias groups: member -> list of names member_to_names: dict[EnumT, list[str]] = {} diff --git a/tests/test_enum.py b/tests/test_enum.py index 99cd048..26def41 100644 --- a/tests/test_enum.py +++ b/tests/test_enum.py @@ -3,6 +3,8 @@ """Tests for the `frequenz.core.enum` module.""" +from typing import Any + import pytest from frequenz.core.enum import ( @@ -58,6 +60,160 @@ def test_deprecated_member_helper_returns_wrapper() -> None: assert isinstance(wrapper, DeprecatedMember) assert wrapper.value == 123 assert wrapper.message == "Value 123 is deprecated" + assert wrapper.new_name is None + assert wrapper.since is None + + structured = deprecated_member(123, new_name="NEW", since="v1.2.3") + assert isinstance(structured, DeprecatedMember) + assert structured.value == 123 + assert structured.message is None + assert structured.new_name == "NEW" + assert structured.since == "v1.2.3" + + +class _RenamedEnum(Enum): + """A test enum with members deprecated in the structured form.""" + + OPEN = 1 + FINISHED = 2 + PENDING = deprecated_member(1, new_name="OPEN", since="v1.2.3") + DONE = deprecated_member(3, new_name="FINISHED") + CLOSED = deprecated_member( + 4, "Closed tasks are gone, use FINISHED", new_name="FINISHED", since="v1.0.0" + ) + + +_RENAMED = f"{__name__}._RenamedEnum" + + +def test_deprecated_member_generates_the_warning() -> None: + """Test that a member with only `new_name` warns with fully qualified names. + + `since` is only for the documentation, so it doesn't show up in the warning. + """ + pending = f"{_RENAMED}.PENDING is deprecated. Use {_RENAMED}.OPEN instead." + done = f"{_RENAMED}.DONE is deprecated. Use {_RENAMED}.FINISHED instead." + + with pytest.deprecated_call() as recorder: + _ = _RenamedEnum.PENDING + _assert_deprecated_member(recorder, pending) + + with pytest.deprecated_call() as recorder: + _ = _RenamedEnum["DONE"] + _assert_deprecated_member(recorder, done) + + with pytest.deprecated_call() as recorder: + _ = _RenamedEnum(3) + _assert_deprecated_member(recorder, done) + + assert _RenamedEnum.__deprecated_names__ == { + "PENDING": pending, + "DONE": done, + "CLOSED": "Closed tasks are gone, use FINISHED", + } + + +def test_deprecated_member_message_wins() -> None: + """Test that a `message` replaces the generated warning, even with `new_name`.""" + with pytest.deprecated_call() as recorder: + _ = _RenamedEnum.CLOSED + _assert_deprecated_member(recorder, "Closed tasks are gone, use FINISHED") + + +def test_deprecated_member_generates_the_warning_for_nested_enums() -> None: + """Test that the generated warning uses the qualified name of the enum.""" + + class _Nested(Enum): + """An enum defined inside a function.""" + + NEW = 1 + OLD = deprecated_member(1, new_name="NEW") + + qualified = f"{__name__}.{_Nested.__qualname__}" + assert ".._Nested" in qualified + with pytest.deprecated_call() as recorder: + _ = _Nested.OLD + _assert_deprecated_member( + recorder, f"{qualified}.OLD is deprecated. Use {qualified}.NEW instead." + ) + + +def test_deprecated_member_needs_a_message_or_new_name() -> None: + """Test that a member saying neither what to warn nor what to use 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 message, new_name or both"): + deprecated_member(1) # type: ignore[call-overload] + with pytest.raises(TypeError, match="needs message, new_name or both"): + deprecated_member(1, None, since="v1.2.3") # type: ignore[call-overload] + with pytest.raises(TypeError, match="needs message, new_name or both"): + DeprecatedMember(1) # type: ignore[call-overload] + + +def test_deprecated_member_signature() -> None: + """Test that `new_name` and `since` are keyword-only.""" + with pytest.raises(TypeError): + deprecated_member(1, None, "NEW") # type: ignore[call-overload] + + +@pytest.mark.parametrize( + "new_name, since, error", + [ + (0, None, TypeError), + ("", None, ValueError), + ("Task.NEW", None, ValueError), + ("NEW", 0, TypeError), + ("NEW", "", ValueError), + ("NEW", " ", ValueError), + ], + ids=[ + "new-name-type", + "new-name-empty", + "new-name-dotted", + "since-type", + "since-empty", + "since-blank", + ], +) +def test_deprecated_member_rejects_bad_arguments( + new_name: Any, since: Any, error: type[Exception] +) -> None: + """Test that `new_name` and `since` are checked when the member is declared.""" + with pytest.raises(error): + deprecated_member(1, new_name=new_name, since=since) + + +def test_deprecated_member_new_name_must_be_a_member() -> None: + """Test that a `new_name` that is not a member of the enum is refused.""" + with pytest.raises(ValueError, match="is not a member of .*_Bad: 'MISSING'"): + + class _Bad(Enum): + """An enum pointing at a member it doesn't have.""" + + NEW = 1 + OLD = deprecated_member(1, new_name="MISSING") + + +def test_deprecated_member_new_name_must_not_be_deprecated() -> None: + """Test that a `new_name` pointing at a deprecated member, itself included, is refused.""" + with pytest.raises(ValueError, match="is 'OLDER', which is deprecated too"): + + class _Chained(Enum): + """An enum pointing at another deprecated member.""" + + NEW = 1 + OLDER = deprecated_member(1, new_name="NEW") + OLD = deprecated_member(1, new_name="OLDER") + + with pytest.raises(ValueError, match="is 'OLD', which is deprecated too"): + + class _Itself(Enum): + """An enum with a member pointing at itself.""" + + NEW = 1 + OLD = deprecated_member(1, new_name="OLD") def test_deprecated_member_helper_enum_usage() -> None: From e118fd8982ab0801f042429d50b2bd73f4755d61 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 23 Oct 2026 09:35:27 +0000 Subject: [PATCH 3/5] Follow the deprecations guide in the enum examples The `Enum`, `unique()` and README examples still wrote messages like `PENDING is deprecated, use OPEN instead`, which the deprecations guide no longer recommends: a warning has to start with the fully qualified name, which the own-deprecations `pytest` filter relies on, and the version belongs in the documentation notice. They now use the structured `deprecated_member()` form for renames, and the `Enum` example also shows a member with no replacement, which gives its own fully qualified `message` and needs a hand-written notice. The text after it says what each one warns and how it is documented. The `Enum` example also claimed that `TaskStatus["DONE"]` and `TaskStatus(3)` were `TaskStatus.FINISHED`, which had a different value, and went unnoticed because the examples are only linted, not run. The new example asserts what actually happens. Signed-off-by: Leandro Lucarella --- README.md | 11 +++++----- src/frequenz/core/enum.py | 42 +++++++++++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 613c64b..ea8f76c 100644 --- a/README.md +++ b/README.md @@ -104,12 +104,13 @@ from frequenz.core.enum import Enum, deprecated_member, unique class TaskStatus(Enum): OPEN = 1 IN_PROGRESS = 2 - # Duplicate values are fine with `@unique` as long as they are deprecated - PENDING = deprecated_member(1, "PENDING is deprecated, use OPEN instead") - DONE = deprecated_member(3, "DONE is deprecated, use FINISHED instead") - FINISHED = 4 + # Duplicate values are fine with `@unique` as long as they are deprecated. + # `since` is for the documentation. + PENDING = deprecated_member(1, new_name="OPEN", since="v1.2.0") -status1 = TaskStatus.PENDING # Warns: "PENDING is deprecated, use OPEN instead" +# Warns ".TaskStatus.PENDING is deprecated. Use +# .TaskStatus.OPEN instead." +status1 = TaskStatus.PENDING assert status1 is TaskStatus.OPEN ``` diff --git a/src/frequenz/core/enum.py b/src/frequenz/core/enum.py index 61d6392..d683662 100644 --- a/src/frequenz/core/enum.py +++ b/src/frequenz/core/enum.py @@ -406,23 +406,43 @@ class Enum(enum.Enum, metaclass=DeprecatingEnumType): class TaskStatus(Enum): OPEN = 1 IN_PROGRESS = 2 - PENDING = deprecated_member(1, "PENDING is deprecated, use OPEN instead") - DONE = deprecated_member(3, "DONE is deprecated, use FINISHED instead") - FINISHED = 4 + FINISHED = 3 + PENDING = deprecated_member(1, new_name="OPEN", since="v1.2.0") + CANCELLED = deprecated_member( + 4, + "mypkg.TaskStatus.CANCELLED is deprecated. " + "Cancelled tasks are deleted now.", + since="v1.3.0", + ) - # Accessing deprecated members: - status1 = TaskStatus.PENDING # Warns: "PENDING is deprecated, use OPEN instead" + # Accessing deprecated members by name warns: + status1 = TaskStatus.PENDING assert status1 is TaskStatus.OPEN - status2 = TaskStatus["DONE"] # Warns: "DONE is deprecated, use FINISHED instead" - assert status2 is TaskStatus.FINISHED + status2 = TaskStatus["PENDING"] + assert status2 is TaskStatus.OPEN - status3 = TaskStatus(1) # No warning, resolves to OPEN which has a non-deprecated alias + # No warning, 1 resolves to OPEN, which is not deprecated + status3 = TaskStatus(1) assert status3 is TaskStatus.OPEN - status4 = TaskStatus(3) # Warns: "DONE is deprecated, use FINISHED instead" - assert status4 is TaskStatus.FINISHED + # Warns, as all the names for 4 are deprecated + status4 = TaskStatus(4) + assert status4.name == "CANCELLED" ``` + + `PENDING` is a plain rename, so `new_name` is enough: reaching it warns + with `mypkg.TaskStatus.PENDING is deprecated. Use + mypkg.TaskStatus.OPEN instead.`, `mypkg` being the module defining + the enum, and + [`griffe-frequenz-core`](https://github.com/frequenz-floss/griffe-frequenz-core) + documents it with a ``Deprecated since v1.2.0. Use + [`OPEN`][mypkg.TaskStatus.OPEN] instead.`` notice. + + `CANCELLED` has no member to point at, so it gives its own `message`, + starting with its fully qualified name. That replaces the warning + only, so its notice is written by hand, as a `Deprecated:` admonition + in the member's docstring. """ __deprecated_names__: ClassVar[Mapping[str, str]] @@ -448,7 +468,7 @@ class TaskStatus(Enum): OPEN = 1 IN_PROGRESS = 2 # This is okay, as PENDING is a deprecated alias. - PENDING = deprecated_member(1, "Use OPEN instead") + PENDING = deprecated_member(1, new_name="OPEN", since="v1.2.0") ``` Args: From b7aebec845b81b2d8e3c393cc7488391b8a45322 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 23 Oct 2026 09:35:54 +0000 Subject: [PATCH 4/5] Update release notes Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index db032e8..2d84434 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -2,18 +2,31 @@ ## Summary - +This release introduces a better way to deprecate enum members. ## Upgrading - +Even when this is a minor release, it ships with 2 potential breaking changes for users of `DeprecatedAlias` and `DeprecatedMember`: + +- The standard warning of a `DeprecatedAlias` with `since` no longer includes the version: it is now ` is deprecated. Use instead.` `since` is only used for the documentation now, following the Frequenz deprecation guide. Update tests matching the old wording. +- `DeprecatedMember.message` is now `str | None`, since a member declared with only `new_name` has its warning generated when the enum class is created. `DeprecatedMember` is not meant to be used directly, `deprecated_member()` is. ## New Features - +- `DeprecatedAlias` now accepts `since` and `message` together, or neither: `since` is only for the documentation, and `message` only replaces the warning. This lets an alias with a custom warning still say since which version it is deprecated in its documentation. +- `deprecated_member()` takes the replacement and version as structured, keyword-only arguments, like `DeprecatedAlias`: `new_name`, the member to use instead in the same enum, and `since`, the version it is deprecated since, for the documentation only. With `new_name` the `message` can be omitted, and the warning is generated with fully qualified names: -## Bug Fixes + ```python + class TaskStatus(Enum): + OPEN = 1 + # Warns "mypkg.TaskStatus.PENDING is deprecated. Use mypkg.TaskStatus.OPEN instead." + PENDING = deprecated_member(1, new_name="OPEN", since="v1.2.0") + ``` + + The enum class checks that `new_name` is one of its members and is not deprecated itself. Existing calls passing a `message` keep working unchanged. - + [`griffe-frequenz-core`](https://github.com/frequenz-floss/griffe-frequenz-core) will read `since` and `new_name` to generate the documentation notice of these members. + +## Bug Fixes - Docstrings now use absolute cross-references to public paths instead of relative ones. Relative cross-references are expanded from the module where an object is defined since mkdocstrings-python 2.0.9, which broke some links, and projects rendering docstrings inherited from this library needed `relative_crossrefs: true` for them to work at all. See [frequenz-repo-config-python#655](https://github.com/frequenz-floss/frequenz-repo-config-python/issues/655) for details. From 14309c851275daf4d7964e0023810d3792da1af6 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Fri, 23 Oct 2026 13:03:30 +0000 Subject: [PATCH 5/5] Add label for the warnings module The `frequenz.core.warnings` module was added in v1.5.0 and the `part:warnings` label already exists in the repository, but neither the labeler, the key labeler nor the bug report template knew about it, so pull requests and issues touching the module were not labeled. Signed-off-by: Leandro Lucarella --- .github/ISSUE_TEMPLATE/bug.yml | 1 + .github/keylabeler.yml | 1 + .github/labeler.yml | 8 ++++++++ 3 files changed, 10 insertions(+) diff --git a/.github/ISSUE_TEMPLATE/bug.yml b/.github/ISSUE_TEMPLATE/bug.yml index aa0b267..3ddbba3 100644 --- a/.github/ISSUE_TEMPLATE/bug.yml +++ b/.github/ISSUE_TEMPLATE/bug.yml @@ -58,6 +58,7 @@ body: - Module utilities (part:module) - Logging utilities (part:logging) - Type hints and typing (part:typing) + - Warnings and deprecation utilities (part:warnings) - Unit, integration and performance tests (part:tests) validations: required: true diff --git a/.github/keylabeler.yml b/.github/keylabeler.yml index 66fb334..082b036 100644 --- a/.github/keylabeler.yml +++ b/.github/keylabeler.yml @@ -24,4 +24,5 @@ labelMappings: "part:tests": "part:tests" "part:tooling": "part:tooling" "part:typing": "part:typing" + "part:warnings": "part:warnings" "part:❓": "part:❓" diff --git a/.github/labeler.yml b/.github/labeler.yml index 9ce001c..d52a7ec 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -132,3 +132,11 @@ - "src/frequenz/core/typing/**" - "tests/test_typing.py" - "tests/typing/**" + +"part:warnings": + - changed-files: + - any-glob-to-any-file: + - "src/frequenz/core/warnings.py" + - "src/frequenz/core/warnings/**" + - "tests/test_warnings.py" + - "tests/warnings/**"