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, 18595 unmerged

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 Simon Charette, 6 months ago

Summary: Switch warnings.warn usage to skip_file_prefixes instead of stacklevelSwitch usage to skip_file_prefixes instead of stacklevel when it makes sense

comment:2 by Natalia Bidart, 6 months ago

Keywords: deprecation stacklevel added
Triage Stage: UnreviewedAccepted
Version: 5.1dev

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 target 5.2 at this point.

comment:3 by Mohammad Salehi, 6 months ago

Hello, If you agree, I would like to review this issue and start working on it ?
Thank you.

comment:4 by Simon Charette, 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 Jae Hyuck Sa , 5 months ago

Owner: set to Jae Hyuck Sa
Status: newassigned

comment:6 by Jae Hyuck Sa , 5 months ago

Has patch: set

comment:7 by Natalia Bidart, 3 months ago

Patch needs improvement: set

comment:8 by JaeHyuckSa, 2 months ago

Owner: changed from Jae Hyuck Sa to JaeHyuckSa
Note: See TracTickets for help on using tickets.
Back to Top