Opened 6 months ago
Last modified 2 months ago
#35667 assigned Cleanup/optimization
Switch usage to skip_file_prefixes instead of stacklevel when it makes sense
Reported by: | Simon Charette | Owned by: | JaeHyuckSa |
---|---|---|---|
Component: | Utilities | Version: | dev |
Severity: | Normal | Keywords: | deprecation stacklevel |
Cc: | Adam Johnson | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Pull Requests: | 18683, | ||
Description ¶
In most of our usages of warnings.warn
(and almost every usage that relates to deprecation warnings) we want them to be associated to the first out-of-as that's the most likely location that can be adjusted to avoid the warning.
In order to achieve this goal we've historically attempted to pass a fixed stacklevel
to warnings.warn
which can sometimes be tricky and error prone depending on how nested and convoluted the user or third-party app offending call site might be. In other cases we've opted not to provide a stacklevel
at all as determining the offending call site under all circumstances is impossible.
Well it appears that this is a problem that Python 3.12 allows frameworks to address in a better way with the introduction of warnings.warn(skip_file_prefixes: tuple[str] | None)
. This new feature would allow us to ensure that the proper offending call site is referenced when emitting from a deeply nested call site where stacklevel
is inappropriate.
Since this is a Python 3.12+ feature I'd suggest we introduce a get_non_django_stacklevel() -> int
(better name welcome) function that could be used to pass to warnings.warn(stacklevel)
until we drop support for Python 3.11.
According to the ticket's flags, the next step(s) to move this issue forward are:
- To improve the patch as described in the pull request review comments or on this ticket, then uncheck "Patch needs improvement".
If creating a new pull request, include a link to the pull request in the ticket comment when making that update. The usual format is:
[https://github.com/django/django/pull/#### PR]
.
Change History (8)
comment:1 by , 6 months ago
Summary: | Switch warnings.warn usage to skip_file_prefixes instead of stacklevel → Switch usage to skip_file_prefixes instead of stacklevel when it makes sense |
---|
comment:2 by , 6 months ago
Keywords: | deprecation stacklevel added |
---|---|
Triage Stage: | Unreviewed → Accepted |
Version: | 5.1 → dev |
comment:3 by , 6 months ago
Hello, If you agree, I would like to review this issue and start working on it ?
Thank you.
comment:4 by , 6 months ago
I think we might want to wait for the patch for #35666 to land before committing efforts here Mohammad.
comment:5 by , 5 months ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:6 by , 5 months ago
Has patch: | set |
---|
comment:7 by , 3 months ago
Patch needs improvement: | set |
---|
comment:8 by , 2 months ago
Owner: | changed from | to
---|
Hello Simon, as always thank you for your detailed and thorough report.
I agree with your proposal, I think this would help reducing the mistakes and overall improve the contributing experience. I'm setting this version to
dev
since this new feature should target5.2
at this point.