A policy on calling super()

[Daniele Procida]

<https://code.djangoproject.com/ticket/21111>

There's some discussion of a particular class, django.views.base.View, and whether its __init__() should contain a super(View, self).__init__().

But there's also a wider question of whether there should be a general rule about this, whether the integrity of the __init__() chain should be maintained, and whether it matters that not all of our classes that are likely to be subclassed do it.

Any comments?

Daniele

[Tom Christie]

Calling super in base classes (ie anything that inherits from 'object') just seems unnecessary and obscure to me.  It's not a pattern I use or have seen, and after playing around a bit I can't see any sensible case where it'd make a difference.  `View` should always be the last (right-most) class in hierarchy, so there shouldn't ever be any parent behaviour that needs calling into.

[Alex Gaynor]

It matters if you're going to mixin a class whose common ancestor is object, e.g.:

class FooMixin(object):

     def __init__(self):

        self.foo = 3

class MyModel(models.Model, FooMixin):

    pass

if models.Model.__init__ doesn't call super().__init__, then FooMixin.__init__ won't be invoked.

Alex

--
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 django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
For more options, visit https://groups.google.com/groups/opt_out.

--

"I disapprove of what you say, but I will defend to the death your right to say it." -- Evelyn Beatrice Hall (summarizing Voltaire)
"The people's good is the highest law." -- Cicero

GPG Key fingerprint: 125F 5C67 DFE9 4084

[Tom Christie]

I'm not sure that's really sensible style, tho - classes that are intended to be used as mixins should call super, and the base class should be the last in the hierarchy.

    class FooMixin(object):

         def __init__(self):

            super(FooMixin, self).__init__()

            self.foo = 3

    class MyModel(FooMixin, models.Model):

        pass

Am I missing some useful case where it's necessary for a mixin class to be used as the base class rather than a super class?

[Aymeric Augustin]

I read the original request as a matter of principle and not something born from an actual need.

View is intended to be the rightmost class in an inheritance scheme. I haven't seen a sane use case for injecting it as a mixin.

This change is more likely to allow beginners to shoot themselves into the foot with an ill-conceived inheritance scheme than the status quo is to prevent advanced and interesting uses.

In general I'm not eager to make constructs without practical value possible. Read this as a -0.

--
Aymeric.

[Glin]

Sorry, I had problems to get to my google account, but finally I'm here :).

I wrote to the ticket meanwhile, but you probably not in CC, so I paste it here, too:

Having this example:

class View(object):
    def __init__(self):
        print "View init"
        #super(View, self).__init__()

class SomeMixin(object):
    def __init__(self):
        print "SomeMixin init"
        super(SomeMixin, self).__init__()

class MyView(View, SomeMixin):
    def __init__(self):
        print "MyVIew init"
        super(MyView, self).__init__()

MyView()
 

With the commented line in the View class, the method SomeMixin.__init__ is not called, because mro chain is broken, that's the reason why I filled the ticket.

Practical use - for exemple in current mixins, there are instance attributes which are not defined in __init__, which is bad, code is harder to read when new attributes pop up on unusual places, every checker like pylint displays warning like:

W:127,8:ModelFormMixin.form_valid: Attribute 'object' defined outside __init__
 

So in ModelFormMixin, there should be __init__ which defines 'object' attribute (self.object = None), which is IMHO more than good practice.

Also having attribute 'object' in __init__, we could avoid ugly constructions like now (also in views/generic/edit.py):

elif hasattr(self, 'object') and self.object is not None:
 

And that's example in Django itself, I bet there could be lot more in various cases of other developers views/mixins, which are hurt by broken mro chain.

[Xavier Ordoquy]

Hi,

I'm jumping in the discussion as I've tried - hard - to do something similar in the past.

However, I'll second what has been said: mixins should be on the left side.

Is there a reason your code can't be similar to:

#class View(object):
#    def __init__(self):
#        print "View init"

class SomeMixin(object):
    def __init__(self):
        super(SomeMixin, self).__init__()
        print "SomeMixin init"

class MyView(SomeMixin, View):
    def __init__(self):
        print "MyVIew init"
        super(MyView, self).__init__()

MyView()

The reason why the mixin has to be left to the view isn't obvious in your example.

If you need the super for the __init__ it's likely that you'll need it for other functions. As opposed to __init__ those functions will not exist in the object class. Therefore you'll end up with testing if the super has the function or not before accessing it because it'll depend on the mixins order.

Regards,

Xavier.

--
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 django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/fb4b4508-99bf-4156-bbdd-f9ee4a957bbd%40googlegroups.com.