Opened 15 months ago
Last modified 6 weeks ago
#35667 assigned Cleanup/optimization
Switch usage to skip_file_prefixes instead of stacklevel when it makes sense
| Reported by: | Simon Charette | Owned by: | Luna |
|---|---|---|---|
| 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 |
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.
Change History (18)
comment:1 by , 15 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 , 15 months ago
| Keywords: | deprecation stacklevel added |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
| Version: | 5.1 → dev |
comment:3 by , 15 months ago
Hello, If you agree, I would like to review this issue and start working on it ?
Thank you.
comment:4 by , 15 months ago
I think we might want to wait for the patch for #35666 to land before committing efforts here Mohammad.
comment:5 by , 14 months ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:6 by , 14 months ago
| Has patch: | set |
|---|
comment:7 by , 11 months ago
| Patch needs improvement: | set |
|---|
comment:8 by , 11 months ago
| Owner: | changed from to |
|---|
comment:9 by , 2 months ago
Just noting we have a someday/maybe ticket to favor importlib.resources over __file__ to better support byte-compiled Django: #30950. Should we be using that when developing this feature?
comment:14 by , 7 weeks ago
| Owner: | removed |
|---|---|
| Status: | assigned → new |
comment:15 by , 7 weeks ago
Now that we're on Python 3.12+ in main this issue is much easier as we don't have to build our own shim and can simply swap usages of stacklevel with skip_file_prefixes.
comment:17 by , 7 weeks ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
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
devsince this new feature should target5.2at this point.