Opened 3 years ago

Closed 2 years ago

#18127 closed Cleanup/optimization (fixed)

Make *all* DeprecationWarnings stacklevel=2 or greater

Reported by: MostAwesomeDude@… Owned by: aaugustin
Component: Uncategorized Version: 1.4
Severity: Normal Keywords:
Cc: garrison@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This is a testing-the-waters ticket, as I don't want to code this up without some discussion.

DeprecationWarning is very useless without sufficient stack information pointing out who is calling the deprecated callable. Many DeprecationWarnings in Django have the default stacklevel of 1, causing the warning to reference the original file:line where the warning was raised, but it would be much more helpful to raise with a stacklevel of 2, pointing to the *caller* who called the deprecated callable.

Even changing just a few of them, however, has caused a massive propagation of warnings while testing, which tells me that there are many places in Django where deprecated code is being called from obstensibly non-deprecated functions. Is there a reason for not placing DeprecationWarnings at the outer edges of Django's interface?

I searched through the tickets for a bit before deciding that nobody had filed this before.

Change History (7)

comment:1 Changed 3 years ago by carljm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

I'm certainly in favor of using stacklevel=2 for deprecation warnings. I don't understand why changing the stacklevel would cause a "massive propagation of warnings" - AFAIU the stacklevel simply changes the reporting of the warning, it shouldn't change how many or which warnings are raised.

In general deprecation warnings are placed at the entry point to a deprecated API - hard to comment more than that without looking at specific examples.

comment:2 Changed 3 years ago by nickmartini

  • Owner changed from nobody to nickmartini

comment:3 Changed 3 years ago by nickmartini

  • Has patch set

All DeprecationWarning are now stacklevel=2. I made no changes to PendingDeprecationWarning.

https://github.com/django/django/pull/349

Last edited 3 years ago by nickmartini (previous) (diff)

comment:4 Changed 3 years ago by garrison

  • Cc garrison@… added

comment:5 Changed 2 years ago by aaugustin

  • Owner changed from nickmartini to aaugustin
  • Status changed from new to assigned

The pull request is out of date, and unfortunately, such patches are easier to redo than to review or update.

The best time to do this is just after we advance the deprecation warnings in master for 1.6. At this point, there won't be any PendingDeprecationWarning left, and as few DeprecationWarning as possible.

comment:6 Changed 2 years ago by aaugustin

The fix for this ticket is included in https://github.com/django/django/pull/605.

comment:7 Changed 2 years ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In ef017a5f00d9e84059e00e8fea123be51b293f75:

Advanced pending deprecation warnings.

Also added stacklevel argument, fixed #18127.

Note: See TracTickets for help on using tickets.
Back to Top