Conversation
github-actions
Bot
added
part:docs
The platforms section claims 20.04, but the runners moved to 24.04, so we update the README to report what is currently actually supported. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Silencing a warning around a call is normally done with a `warnings.catch_warnings` block, but entering and leaving one bumps CPython's internal filters version, which invalidates the `__warningregistry__` of every loaded module. Every warning already shown in the program is then shown again, on every call, including warnings emitted by unrelated code (python/cpython#73858, open since 2017). For a converter that silences a deprecation once per message, that turns a warning shown once into one shown for every message that goes by. This new utilty adds the filter to the list the interpreter consults and takes it out again on exit, so the version never moves and no registry ever goes stale. That is sound only for "ignore", which is all this offers: the registry is consulted before the filters, and an ignore can only take warnings away, so a warning recorded as shown on either side of the block is still correctly recorded on the other. `simplefilter()` and `filterwarnings()` bump the version on purpose, because they can also add warnings back. Repairing the registries afterwards is not affordable: it has to scan `sys.modules` and copy every registry it finds, measured at 82 µs per block with 282 modules loaded and ~130 µs with ~370 in a dispatch client field test, against 1.3 µs for the standard library block and 1.9 µs for this. A test suite went from 2.8 s to 12 s on it. Two hazards are handled on exit: 1. Code in the block can replace `warnings.filters` with a copy, which carries our entry. If the lists differ, the entry is removed from both: leaving it in the live one keeps the ignore active, and leaving it in the original lets a `catch_warnings` block restore it. 2. Only our entry is touched. An entry/exit snapshot would make blocks overlapping across threads (A enters, B enters, A exits, B exits) restore each other's stale copies, leaving an ignore installed for good, as the new test shows. So changes made inside the block aren't undone, and the docstring says this is no `catch_warnings` replacement. Re-entering an instance inside its own block raises `RuntimeError` before touching anything, as it would add two filters and remove one. Supporting it not worth it. Removal is a lookup plus a delete by index, so a thread mutating the list concurrently can make it delete the wrong filter. A lock would cost every block to protect only programs changing filters from several threads, but threads are still poorly supported in Python, so we skip it and warn about it in the docs for now. Nor is the ignore per task or thread: the filters are shared, so a block held across an `await` also ignores other tasks' warnings, and an outer `catch_warnings` uses the same state. Python 3.14's `-X context_aware_warnings` doesn't help either: 1. only a `catch_warnings` block creates a new context; 2. a task inherits the filters of the task that started it; 3. outside such a block, the process-wide filters apply. Isolation would need our own context, possible only from 3.14 with the flag on: with it off, the C code reads the context but the Python API writes the module global, so a `catch_warnings` inside records nothing. The scope note explains this, and a test pins down the sibling task case. As the entry is inserted directly, the arguments `warnings.filterwarnings()` checks are checked here too, with real exceptions (it uses `assert`s up to 3.12). A bad pattern raises `re.error`. If the filters in effect can't be reached, it falls back to a standard library block, the old behavior. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Silencing the deprecations a library raises on itself is the case that motivated the module, and the one that will appear at dozens of call sites, so it gets a name instead of having to use `ignoring_warnings(DeprecationWarning)` every time. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
PEP 702 rejected deprecating modules, attributes and constants, and rejected a `Deprecated[T, message]` modifier along with them, so no type checker reports a use of one of these names, and a type alias even launders a deprecation that does exist. This commit introduces `deprecated_aliases()` to overcome this, allowing to define a table of symbols in a module that are deprecated aliases for a symbol new location. A module `__getattr__` is needed for this because the symbol's new location can't be marked as deprecated, it would deprecate it for everybody, including the users of its new home. Serving the object itself, rather than a wrapper, is what keeps `isinstance` working through both import paths, and looking it up lazily keeps the old module from importing the new one just to hold a reference. The alias table is checked and copied when it is declared, rather than left for the lookup to trip over. The usage is a bit tricky/hacky, so it is properly documented. The module's `__getattr__` assignment must be in a `else:` branch of the `if TYPE_CHECKING:` block that declares the aliases, never at module level. Not doing so is problematic because mypy treats a module with a `__getattr__` as `dict[str, Any]`, so all symbols in the module suddenly lose all type annotations. So the assignment must stay out of mypy's eyes. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
A symbol that moves to another module might also be renamed on the way, and so far the alias could only keep the name it had. The target string now takes an optional `:name` suffix, as an entry point does: `"fractions:Fraction"` next to the plain `"decimal"`. The suffix is checked along with the rest of the table, since `partition()` stops at the first colon: `"a:b:c"` would otherwise pass as module `a` and name `b:c`, and the deprecation would tell the user to reach for `a.b:c`. An empty module or an empty name after the colon is refused there too. Taking the string apart once, when the table is declared rather than on every lookup, is what makes that possible. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
The default message says where the symbol went and nothing else, which
is enough for a plain move but not when the deprecation comes with a
removal version, a link to a migration guide, or a category other than
`DeprecationWarning` (`FutureWarning` for something aimed at end users,
a library's own category so it can be filtered on its own).
So `message` is a template formatted with the old and new fully
qualified names, `category` is the warning class, and `stacklevel` is
there for the rare case of the returned `__getattr__` being wrapped.
All three are checked where the alias table already is, when the aliases
are declared. The template is checked by formatting it once with empty
names, which is the only way to catch a stray brace: a message carrying
something like `{'a': 1}` otherwise raises a `KeyError` from inside
`warnings.warn()`, at the lookup, pointing at neither the template nor
the call that set it.
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
The usual way to assert that a call doesn't warn is an `"error"` filter, which is also what pytest documents since `pytest.warns(None)` was removed in pytest 8. But it makes `warnings.warn()` raise inside the code under test, where a broad `except` can swallow it or turn it into an unrelated failure, and the failure points at the `warn()` call instead of the call the test is about. This records instead. Matching warnings are reported together, each with its message and `filename:lineno`. The `"always"` filter added inside matches on message as well as category, so a warning already shown elsewhere is still caught, while other warnings keep whatever the surrounding filters say (ignored, deduplicated or raised). Recording needs `showwarning` reset to the default, and `catch_warnings(record=True)` only does that in process-wide mode. With context-aware warnings (3.14, default in free-threaded builds), a handler installed by the application, like `logging.captureWarnings()`, keeps taking every warning and nothing is recorded, so the assertion passes silently (python/cpython#151149, which also breaks `pytest.warns`). So the reset is done here in every mode, and undone before recording ends so the replay reaches the application's handler. A handler the code under test installs inside the block still wins. On exit, every warning not turned into a failure is replayed, since recording is the only reason it wasn't shown. If an exception is propagating, it takes precedence over the assertion and the matching warnings are replayed too, so a deprecation inside `pytest.raises()` isn't lost. Replay goes through `_showwarnmsg()`: the public `showwarning()` drops the `source` of a `ResourceWarning`, and the default implementation under it ignores an application's `showwarning()`, sending logged warnings to stderr. Unlike `ignoring_warnings()`, this resets the deduplication history. It has to: a warning already shown is rejected by `__warningregistry__` before the filters run, and only bumping the filters version, as `catch_warnings()` does, invalidates it. For tests, this cost is reasonable, but the docstring says to keep it out of hot paths and long-running processes. Other choices: - No `module` argument: a recorded `WarningMessage` has a filename, not the module name the filter matched, so the report couldn't reproduce the filter's decision. A comment explains this. - Recorded warnings are sorted in a single pass, since an `"always"` filter in a loop can record tens of thousands of them. - Re-entering an instance raises `RuntimeError`, as in `ignoring_warnings()`. Otherwise the inner entry would replace the outer recording, and the assertion would silently pass. Known limits, documented in the docstring: - Without context-aware warnings recording is process-wide, so a matching warning from another thread, or from another task across an `await`, fails the block (a test covers this). With them, new threads escape it, but tasks started inside still land here. - An `ignoring_warnings()` inside the code under test wins over this filter, so the assertion covers only what the code didn't silence. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Checking that the replacement for a deprecated symbol doesn't itself go through the deprecated one is the case these assertions will be written for most of the time, so it gets a name instead of `asserting_no_warnings(category=DeprecationWarning)` spelled out at every call site, the same way `ignoring_deprecations()` shortens the other block. `message` is forwarded, and is the only way to narrow this one down: the block wraps the code being tested, so unlike an `ignoring_deprecations()` block it can't be made more precise by making it shorter. That matters when the code under test legitimately deprecates something else, or reaches a third-party deprecation there is nothing to be done about. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Every `deprecated_aliases()` test so far builds a `ModuleType` and assigns the returned function onto it. That covers the function but not the `if TYPE_CHECKING:` and `else:` structure the docstring tells people to write, or a module the import system actually loaded, nor `__all__`. The pattern could stop working and the tests would stay green. Add `tests/warnings/documented_aliases/` for the new tests. The new tests check what the documentation promises: the alias is the very same object as its target, reaching it warns with the name of its new home, a name the package defines itself is untouched, an unknown name still raises `AttributeError`, and `__all__` with a wildcard import pulls the aliases through, warning on the way. The wildcard test asserts the set of messages and not their number, because a package is asked for each name in `__all__` twice, once by the import machinery finding out whether it names a submodule and once by the wildcard import itself. That is CPython's business, not this module's. The `type: ignore[attr-defined]` on the unknown name is the whole point of the `else:` branch rather than a wart: with the `__getattr__` at module level mypy would type that access `Any` and say nothing at all. The package sits next to the tests that import it and is reached as `tests.warnings.documented_aliases`, which is the name both pytest and mypy know it by, since `tests/` and `tests/warnings/` are packages. That is also why the deprecation messages it raises carry that whole path. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
`deprecated_aliases()` can keep a symbol reachable from its old module, and nothing more. A submodule that moved is out of its reach, because the import system never consults a package's `__getattr__`: with a table mapping `sub` to `newpkg:sub`, `import old.sub` and `from old.sub import X` both raise `ModuleNotFoundError`, and only `from old import sub` goes through the alias, and then only when the new package imports the submodule in its own `__init__.py`. Serving it anyway is the worst of the three outcomes. A user who writes the one form that works ships code that breaks the moment somebody else writes either of the other two, and the error they get says nothing about the alias table that promised the move. So resolving an alias onto a module now raises `TypeError` naming the alias and saying what to do instead, which is to keep a real `__init__.py` at the old path with the alias table in its body. The check is at resolution and not at declaration, unlike everything else checked here. Telling a module from a name at declaration time means asking `importlib.util.find_spec()`, which imports the target's parent package to answer, and the laziness that buys is the reason the helper exists. Resolution costs nothing and fires the first time anything reaches the alias, which in these repositories is their own deprecation test. The target is resolved before the warning is raised now, for the same reason: an alias that can't work should say so rather than first announce a move that didn't happen, and under an `"error"` filter that warning would raise and hide the `TypeError` entirely. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
`requires-python` says `>= 3.11, < 4`, but the matrix stopped at 3.12, so 3.13 and 3.14 were shipped untested. The new `warnings` module makes that expensive: it reaches for `warnings._get_filters()`, `_showwarning_orig` and `_showwarnmsg()`, private names that only exist or only matter on those versions, and its one test for context-aware warnings carries a `skipif(sys.version_info < (3, 14))`, so it never ran anywhere. The fallbacks around those names are silent by design, which is what makes the gap worth closing rather than noting: if `_showwarning_orig` goes away, `asserting_no_warnings()` stops recording under `logging.captureWarnings()` and every assertion passes, and nothing says so. The README claimed 3.11 alone under the platforms it is tested on, which was true of the matrix and not of `requires-python`; it now lists the four. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
The `addopts` line predates the current `frequenz-repo-config` template, which has no `addopts` at all and says the same thing through `filterwarnings`, an `"error"` default with `"once"` for the two deprecation categories. Expressing it as a list also makes room for the per-warning exceptions a repository eventually needs, which a command line string can only carry as more `-W` flags. The policy itself is unchanged and worth restating: deprecations must never be errors, here or downstream, so they are shown rather than raised. `"once"` is per message and category for the whole run, where the old `"default"` was per location, which is why the test module builds its warning messages out of the module name. The `-vv` goes with it, separately: the template dropped it, and repo-config carried a `migrate.py` step to take it out of existing repositories, which this one never ran. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
The README lists what the library provides and carries a short example per module, `enum` and `typing` included, so a new module that appears only in the generated API reference is invisible from the landing page. The examples are the three entry points, each showing the thing that makes it worth having rather than its full shape: the deduplication history that survives an `ignoring_deprecations()` block, an alias table with a rename in it, and an assertion that a call doesn't warn. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
llucax
force-pushed
the
warnings-utilities
branch
from
September 24, 2026 13:00
76f5999 to
ef9f279
Compare
llucax
marked this pull request as ready for review
September 24, 2026 13:00
llucax
requested review from
Marenz,
daniel-zullo-frequenz and
shsms
and removed request for
a team
September 24, 2026 13:00
Contributor
Author
|
I already went through 4 internal reviews in this branch, and stopped because I got tired. I will trigger a copilot review, and I'm pretty sure it will find more issues. I'm more inclined now to live with the limitations, I think it should be useful in its current state, but let's see. The commit messages have a lot of extra information, I recommend reviewing on a commit-by-commit basis. I'm thinking of making __getattr__ = deprecated_aliases(
__name__,
DeprecatedAlias(
"Decimal",
target="whatever.decimal",
message="{old} is deprecated since v1.2.3, use {new} instead"),
)
DeprecatedAlias(
"Rational",
target="whatever.fractions:Fraction",
message="{old} is deprecated since v1.4.3, use {new} instead"),
)
)But will do it in a follow-up commit, this PR is already long and horrifying enough 😆 |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The context-aware warning scope documentation is inaccurate and should be corrected before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds warning-management utilities for suppression, assertions, and deprecated aliases, with comprehensive tests and documentation.
Changes:
- Adds five public warning and deprecation helpers.
- Covers warning filtering, replay, concurrency limitations, and alias validation.
- Expands CI support through Python 3.14.
| File | Description |
|---|---|
src/frequenz/core/warnings/__init__.py |
Exports the new public API. |
src/frequenz/core/warnings/_asserting.py |
Implements warning assertions and replay. |
src/frequenz/core/warnings/_deprecated_aliases.py |
Implements deprecated module aliases. |
src/frequenz/core/warnings/_ignoring.py |
Implements warning suppression. |
tests/warnings/test_asserting.py |
Tests warning assertions and replay. |
tests/warnings/test_blocks.py |
Tests shared context-manager behavior. |
tests/warnings/test_deprecated_aliases.py |
Tests deprecated aliases. |
tests/warnings/test_ignoring.py |
Tests warning suppression and history. |
tests/warnings/conftest.py |
Provides warning-test fixtures. |
tests/warnings/documented_aliases/__init__.py |
Provides a realistic alias package. |
tests/warnings/__init__.py |
Defines the warning-test package. |
tests/__init__.py |
Makes test helpers importable consistently. |
README.md |
Documents the new utilities and platforms. |
RELEASE_NOTES.md |
Announces the new module. |
pyproject.toml |
Updates pytest warning filters. |
.github/workflows/ci.yaml |
Adds Python 3.13 and 3.14 coverage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Danger: | ||
| Follow the usage example structure strictly. In particular never drop | ||
| the `else:`, otherwise a type checker will see the `__getattr__` | ||
| assignement and treat every symbol in the module as [`Any`][typing.Any], |
Comment on lines
+135
to
+141
| Nothing changes that, not even wrapping this in a | ||
| [`warnings.catch_warnings`][] block, which reaches for the same shared | ||
| state. Python 3.14's `sys.flags.context_aware_warnings` doesn't help | ||
| either: it lets [`warnings.catch_warnings`][] keep the filters in a | ||
| [`contextvars.ContextVar`][], but a task inherits the list the task that | ||
| started it was using rather than getting one of its own, and outside such | ||
| a block the filters in effect are the process-wide ones anyway. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a new module
frequenz.core.warningswith five public symbols to work around Python limitations when dealing with deprecations (and some generalized to all warnings).Warning
This is tricky code, full of edge cases. Every utility here reaches into CPython's warnings machinery, either because the documented API has a bug or because it has no negative form. The docstrings are long for the same reason: each
Warning:section is a failure mode that was reproduced.catch_warnings()works by setting globals, so it is not thread-safe norasync-safe. Another task could get its warnings affected or affect the code in thecatch_warnings()block. This was solved in Python 3.14 only, but as an opt-in feature (the context_aware_warnings flag can be set the -X context_aware_warnings command-line option or by the PYTHON_CONTEXT_AWARE_WARNINGS environment variable, but it is off by default). Making everything quite messy and leaving some issues unsolvable.ignoring_warnings()silences warnings around a block without resetting the program's deduplication history, whichwarnings.catch_warningsdoes on every entry and exit (python/cpython#73858, open since 2017).asserting_no_warnings()fails a test when the block raises a matching warning, by recording rather than by makingwarnings.warn()raise inside the code under test. Again, trying to work-aroundcatch_warningsreset issues.deprecated_aliases()builds a module__getattr__that keeps the old import path of a symbol that moved, to solve some@deprecateddecorator limitations (PEP 702 rejected deprecating modules, attributes and constants.ignoring_deprecations()andasserting_no_deprecations()are the shortcuts for the common case.To accompany some of these new utilities, a new Griffe extension,
griffe-frequenz-corewas built somkdocscan automatically document deprecated symbols as such when they are deprecated via the newdeprecated_aliases(), and the existingenum.deprecated_member(). This complements the existinggriffe-warnings-deprecatedthat does that for the standard@deprecateddecorator.