Unfortunately this would break in subtle in confusing ways in inheritance
scenarios.
Say you've defined the following Manager subclass prior to Django 1.6 and
we're only raising a warning on Manager.get_query_set calls.:
class ActiveManager(models.Manager):
def get_query_set(self, *args, **kwargs):
return super(ActiveManager, self).get_query_set(*args,
**kwargs).filter(is_active=True)
You would get a Manager.get_query_set() is deprecated, use
`get_queryset()`instead.warning upon calling
ActiveManager.get_query_set, so far so good.
>From here you would go ahead and rename your super call and/or rename
ActiveManager.get_query_set to get_queryset:
class ActiveManager(models.Manager):
def get_query_set(self, *args, **kwargs):
return super(ActiveManager, self).get_queryset(*args,
**kwargs).filter(is_active=True)
Would be broken since internal code calls get_queryset. And renaming both:
class ActiveManager(models.Manager):
def get_queryset(self, *args, **kwargs):
return super(ActiveManager, self).get_queryset(*args,
**kwargs).filter(is_active=True)
Would not be backward compatible with code not yet migrated (third-party
apps for example) that still calls get_query_set. The correct way of
handling this *by hand* would be to define both methods when you override a
renamed one:
class ActiveManager(models.Manager):
def get_queryset(self, *args, **kwargs):
return super(ActiveManager, self).get_queryset(*args,
**kwargs).filter(is_active=True)
def get_query_set(self, *args, **kwargs):
return ActiveManager.get_queryset(self, *args, **kwargs)
Again we could raise a more verbose warning telling the user to do exactly
that but that would break if you're subclassing a Manager subclass from a
third party app that didn't migrate yet:
class ThirdPartyManager(models.Manager):
"""You cannot do the rename here since it's from a third party
(site-packages,...) module"""
def get_query_set(self, *args, **kwargs):
qs = super(ThirdPartyManager, self).get_query_set(*args, **kwargs)
# To something with qs...
return qs
And you would be left to yourself figuring out you have to do the following:
class MyManager(ThirdPartyManager):
def get_queryset(self, *args, **kwargs):
# Call ThirdPartyManager.get_query_set because it didn't migrate yet.
qs = super(MyManager, self).get_query_set(*args, **kwargs)
# To something with qs...
return qs
def get_query_set(self, *args, **kwargs):
# Compatiblity shim
return MyManager.get_queryset(self, *args, **kwargs)
Maintaining backward compatiblity with third party apps is the main reaosn
a class decorator wouldn't work here. @python_2_unicode_compatible works
well because __str__ and __unicode__ are not often involved in supertrickery
while you almost always override
get_queryset when you subclass Manager.
The use of the metaclass make sure you're not shooting yourself in the foot
all and all the metaclass subclassing trouble should only concern a
minority of users who most probably know what they're doing.
Simon
Le mercredi 6 mars 2013 10:30:35 UTC-5, Jacob Kaplan-Moss a écrit :
>
> Hm.
>
> I'm +1 on cleaning up the names.... but do we *really* have to use a
> metaclass for this? Seems to me this is "gratuitous use of a
> metaclass" territory, especially given the shenanigans you have to go
> through with subclassing the metaclass.
>
> I'd be a lot happier with this patch if it just did this in a simple
> manner: have the old methods raise the warnings and call the new ones.
> Unless there's a actual need for the metaclassing beyond saving a few
> lines of code can you drop it, please?
>
> Jacob
>
> On Tue, Mar 5, 2013 at 2:20 AM, charettes <[email protected]<javascript:>>
> wrote:
> > Since the consensus as shifted toward not making a break-everything 2.0
> I'm
> > planning to normalize the queryset methods name early in the 1.6
> development
> > cycle to spot any breakages.
> >
> > The affected classes are the following:
> >
> > django.contrib.admin.options.BaseModelAdmin (queryset -> get_queryset).
> > django.contrib.admin.views.main.ChangeList (get_query_set ->
> get_queryset).
> > django.contrib.comments.templatetags.comments.BaseCommentNode
> (get_query_set
> > -> get_queryset).
> > django.contrib.contenttypes.generic.GenericForeignKey
> > (get_prefetch_query_set -> get_prefetch_queryset).
> > django.db.models.fields.related.SingleRelatedObjectDescriptor
> (get_query_set
> > -> get_queryset, get_prefetch_query_set -> get_prefetch_queryset).
> > django.db.models.fields.related.ReverseSingleRelatedObjectDescriptor
> > (get_query_set -> get_queryset, get_prefetch_query_set ->
> > get_prefetch_queryset).
> > django.db.models.manager.Manager (get_query_set -> get_queryset,
> > get_prefetch_query_set -> get_prefetch_queryset).
> >
> > And subclasses (ModelAdmin, ...) where bold items are part of the
> documented
> > API and "->" stands for rename.
> >
> > The patch does the following:
> >
> > Define the new method if missing and complain about it.
> > Define the old method if missing to assure backwards compatibility.
> > Complain whenever an old method is called.
> >
> > As discussed in the ticket the whole MRO is handled by the patch so
> mixins
> > should also work correctly. The only issue you might encounter is if you
> > defined a custom metaclass for one of the classed affected by the
> rename.
> > The simple fix is to make sure your metaclass subclasses the class of
> the
> > class you're planning to attach it to:
> >
> > class MyManagerBase(Manager.__class__):
> > pass
> >
> > class MyManager(Manager):
> > __metaclass__ = MyManagerBase
> >
> >
> > If your code base or library must support both Django > 1.6 and <= 1.6
> you
> > should define both methods until you drop support for < 1.6:
> >
> > class MyManager(Manager):
> > def get_queryset(self, *args, **kwargs):
> > # Do something funky
> > # ...
> >
> > def get_query_set(self, *args, **kwargs):
> > # TODO: Remove when support for Django < 1.6 is dropped.
> > return MyManager.get_queryset(self, *args, **kwargs)
> >
> >
> > The pull request can be reviewed here .
> >
> > I'm planning to commit the proposed fix this week if no major issues are
> > raised here or on the ticket.
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups
> > "Django developers" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an
> > email to [email protected] <javascript:>.
> > To post to this group, send email to
> > [email protected]<javascript:>.
>
> > Visit this group at
> http://groups.google.com/group/django-developers?hl=en.
> > For more options, visit https://groups.google.com/groups/opt_out.
> >
> >
>
--
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/django-developers?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.
