Opened 5 years ago

Closed 4 years ago

#18127 closed Cleanup/optimization (fixed)

Make *all* DeprecationWarnings stacklevel=2 or greater

Reported by: MostAwesomeDude@… Owned by: Aymeric Augustin
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 5 years ago by Carl Meyer

Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/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 4 years ago by nickmartini

Owner: changed from nobody to nickmartini

comment:3 Changed 4 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 4 years ago by nickmartini (previous) (diff)

comment:4 Changed 4 years ago by Jim Garrison

Cc: garrison@… added

comment:5 Changed 4 years ago by Aymeric Augustin

Owner: changed from nickmartini to Aymeric Augustin
Status: newassigned

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 4 years ago by Aymeric Augustin

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

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

Resolution: fixed
Status: assignedclosed

In ef017a5f00d9e84059e00e8fea123be51b293f75:

Advanced pending deprecation warnings.

Also added stacklevel argument, fixed #18127.

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