Code

Opened 7 months ago

Closed 6 weeks ago

#21188 closed Cleanup/optimization (fixed)

Use DeprecationWarning subclasses for deprecated features

Reported by: akaariai Owned by: nobody
Component: Core (Other) Version: master
Severity: Normal Keywords:
Cc: adi@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by timo)

The idea is to introduce new deprecation subclass for each Django version. For example, in current master:

class ToBeRemovedInDjango19Warning(PendingDeprecationWarning):
    pass


class ToBeRemovedInDjango18Warning(DeprecationWarning):
    pass

By using these warnings we can achieve the following:

  • Moving PendingDeprecationWarning to DeprecationWarning after release is single line change.
  • It will be possible to filter on Django's deprecation warnings without catching warnings from non-django components.
  • It will be harder to miss removal of code. After 1.7 is released just remove ToBeRemovedInDjango18Warning. Any code importing that warning will immediately fail. Also, grepping might be a little easier.
  • After release, currently introducing a new PendingDeprecationWarning before PendingDeprecationWarnings are renamed to DeprecationWarnings (as part of release procedure) the new deprecatin will likely be moved to DeprecationWarning. But if a ToBeRemovedInDjango20Warning is added it will be really hard to accidentally move that one to DeprecationWarning.
  • It will be technically easy to provide for longer than two releases deprecation periods (if we ever want this).

Drawbacks:

  • If we use automatic filtering for deprecations, then code currently doing:

warning.simplefilter("always", DeprecationWarning)

will no longer catch Django's deprecation warnings (as we added an entry for warning.simplefilter("default" ToBeRemovedInDjango18Warning) which matches before DeprecationWarning.

I tried this approach in: https://github.com/akaariai/django/compare/deprecation_subclasses#L3R18 - The patch is totally incomplete so I will not mark "has patch".

Attachments (0)

Change History (9)

comment:1 Changed 7 months ago by timo

  • Description modified (diff)

comment:2 Changed 7 months ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

Fancy!

comment:3 Changed 7 months ago by aaugustin

To increase perceived urgency, I suggest using DeprecatedInDjango16 for features that go away in Django 1.8, etc.

comment:4 Changed 7 months ago by akaariai

If using DeprecatedInDjango16 instead of ToBeRemovedInDjango18 then it will be impossible to use shorter or longer deprecation periods. If I am not mistaken shorter periods have been used sometimes in the past.

ToBeRemovedInDjango18Warning isn't a good name, so something better is welcome...

comment:5 Changed 6 months ago by ajs

how about DeprecationInDjango18Warning. That keeps the same context that is used now and just adds a time when it will happen.

comment:6 Changed 6 months ago by ajs

  • Cc adi@… added

comment:7 Changed 7 weeks ago by claudep

  • Has patch set

This pull request should be almost complete: https://github.com/django/django/pull/2376

comment:8 Changed 7 weeks ago by charettes

  • Triage Stage changed from Accepted to Ready for checkin

Reviewed the PR by moving warnings subclasses from django.core.exceptions to django.utils.deprecation and it seems to be completed.

I ran the full test suite against the {Py2, Py3} X {SQLite3, PostgreSQL} matrix with python -Wonce and didn't get any failures or warnings (except the already existing wizard ones on Py3).

comment:9 Changed 6 weeks ago by Claude Paroz <claude@…>

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

In 210d0489c5daad56b806f8165f9fe09fb3c2a019:

Fixed #21188 -- Introduced subclasses for to-be-removed-in-django-XX warnings

Thanks Anssi Kääriäinen for the idea and Simon Charette for the
review.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.