Opened 13 years ago
Closed 12 years ago
#18127 closed Cleanup/optimization (fixed)
Make *all* DeprecationWarnings stacklevel=2 or greater
Reported by: | 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 by , 13 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Uncategorized → Cleanup/optimization |
comment:2 by , 12 years ago
Owner: | changed from | to
---|
comment:3 by , 12 years ago
Has patch: | set |
---|
All DeprecationWarnings are now stacklevel=2. I made no changes to PendingDeprecationWarnings.
comment:4 by , 12 years ago
Cc: | added |
---|
comment:5 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → 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 by , 12 years ago
The fix for this ticket is included in https://github.com/django/django/pull/605.
comment:7 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
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.