5

I'm overwriting the save method of a ModelForm and I don't know why it would cause recursion:

@parsleyfy
class AccountForm(forms.ModelForm):
    def save(self, *args, **kwargs):
        # some other code...
        return super(AccountForm, self).save(*args,**kwargs)

Causes this:

maximum recursion depth exceeded while calling a Python object

Stacktrace shows this line repetitively calling itself:

return super(AccountForm, self).save(*args,**kwargs) 

Now, the parsley decorator is like this:

def parsleyfy(klass):
    class ParsleyClass(klass):
      # some code here to add more stuff to the class
    return ParsleyClass

As @DanielRoseman suggested that the Parsley decorator extending the AccountForm causes the super(AccountForm,self) to keep calling itself, what's the solution?

Also I cannot get my head around this why this would cause recursion.

slava
  • 1,901
  • 6
  • 28
  • 32
James Lin
  • 25,028
  • 36
  • 133
  • 233
  • 1
    I dont think you are supposed to return the parent save() method, just do super(AccountForm, self).save(*args,**kwargs) – PepperoniPizza Feb 06 '13 at 22:12
  • 1
    Are you sure that's your actual code? This is usually what happens if you've mistakenly referred to the superclass in the `super` call - eg you actually have a subclass of AccountForm, and in that overridden save method you're still calling `super(AccountForm...)`. – Daniel Roseman Feb 06 '13 at 22:34
  • @PepperoniPizza you should *definitely* return a modelform super save as it is expected to return an instance – Yuji 'Tomita' Tomita Feb 06 '13 at 23:50
  • @JamesLin What does `ParsleyClass` do? Can you provide a more complete example? – Austin Phillips Feb 07 '13 at 03:37
  • @AustinPhillips it's a decorator that extends the class being decorated and adds a few stuff to the base class. – James Lin Feb 07 '13 at 04:46
  • @JamesLin: Is there a particular reason you need to override `.save()` here in the first place? (Are you putting other logic in it? The code you have right now is pointless; you could just omit the override entirely.) – Amber Feb 07 '13 at 05:02
  • @Amber there are actually other code in the save function, I omitted them out because they are not causing the problem, but I have updated the code to present the reason I need to overwrite it. – James Lin Feb 07 '13 at 05:05

2 Answers2

6

What you could do is just call the parent's method directly:

@parsleyfy
class AccountForm(forms.ModelForm):
    def save(self, *args, **kwargs):
        # some other code...
        return forms.ModelForm.save(self, *args,**kwargs)

This should neatly avoid the issue introduced by your class decorator. Another option would be to manually call the decorator on a differently named base class, rather than using @ syntax:

class AccountFormBase(forms.ModelForm):
    def save(self, *args, **kwargs):
        # some other code...
        return super(AccountFormBase, self).save(*args,**kwargs)

AccountForm = parsleyfy(AccountFormBase)

However, you might also want to consider using a pre-save signal instead, depending on what you're trying to do - it's how one normally adds functionality that should happen before the rest of the model save process in Django.


As for why this is occurring, consider what happens when the code is evaluated.

First, a class is declared. We'll refer to this original class definition as Foo to distinguish it from the later class definition that the decorator will create. This class has a save method which makes a super(AccountForm, self).save(...) call.

This class is then passed to the decorator, which defines a new class which we'll call Bar, and inherits from Foo. Thus, Bar.save is equivalent to Foo.save - it also calls super(AccountForm, self).save(...). This second class is then returned from the decorator.

The returned class (Bar) is assigned to the name AccountForm.

So when you create an AccountForm object, you're creating an object of type Bar. When you call .save(...) on it, it goes and looks up Bar.save, which is actually Foo.save because it inherited from Foo and was never overridden.

As we noted before, Foo.save calls super(AccountForm, self).save(...). The problem is that because of the class decorator, AccountForm isn't Foo, it's Bar - and Bar's parent is Foo.

So when Foo.save looks up AccountForm's parent, it gets... Foo. This means that when it tries to call .save(...) on that parent, it actually just winds up calling itself, hence the endless recursion.

Amber
  • 507,862
  • 82
  • 626
  • 550
  • What's the difference between super(AccountForm,self).save vs forms.ModelForm.save? I thought the save method is instance method and you cannot call it like that? – James Lin Feb 07 '13 at 05:19
  • @JamesLin You can call it as long as you manually provide it an instance of the class as the first argument. As for what the difference is, see the second half of my answer. – Amber Feb 07 '13 at 05:23
  • 1
    @JamesLink I also added a note about [pre-save signals](https://docs.djangoproject.com/en/dev/ref/signals/#pre-save), which are Django's preferred way of adding functionality that triggers immediately before a model save. – Amber Feb 07 '13 at 05:28
  • 2
    Another solution to the issue would be to give the original class a different name, then manually call the decorator to create the class you intend to use (e.g. `AccountForm`). As long as the `super` call used the name of the original class (and not the name of the decorated version), it should work as intended. – Blckknght Feb 07 '13 at 05:31
  • @Amber So if forms.ModelForm.save(self, *args, **kwargs) actually achieve the same thing as super(AccountForm, self).save(), why it is not generally recommended to use the first form? Looks like it will prevent a lot of situations like this. – James Lin Feb 07 '13 at 05:37
  • @Blckknght Yep, that would be another way of working around the issue. – Amber Feb 07 '13 at 05:37
  • @Blckknght would you mind giving an answer with code to elaborate a bit more? – James Lin Feb 07 '13 at 05:38
  • 1
    @JamesLin There are other problems that can arise from calling the parent's method directly in some scenarios - especially those that involve things like calling *other* methods on the same class that may have been overridden. In single-inheritance cases, these issues don't tend to pop up, but multiple inheritance can get really tricky. – Amber Feb 07 '13 at 05:38
  • 1
    @JamesLin I've expanded my answer with an example of Blckknght's suggestion. – Amber Feb 07 '13 at 05:43
  • 1
    @JamesLin The intricacies of `super()` are far too complex to really fit in a single comment thread; I'd suggest reading through http://www.artima.com/weblogs/viewpost.jsp?thread=236275 and http://docs.python.org/2/library/functions.html#super if you really want to know more. The short story is that the concept of 'parent' becomes a lot more complex when you're using multiple inheritance (multiple superclasses, e.g. `class C(A, B):`). – Amber Feb 07 '13 at 05:45
  • @Amber Doesn't this whole thing contradicts the decorator purpose? I mean the code being decorated should not be aware of decorators. – James Lin Feb 07 '13 at 06:35
  • Welcome to the joys of inheritance. – Amber Feb 07 '13 at 06:40
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/24094/discussion-between-james-lin-and-amber) – James Lin Feb 07 '13 at 06:47
  • (I am the author of the lib). I changed the decorator to reset the `__bases__` to `klass.__bases__`. Do you think this will cause some other problems? – shabda Feb 07 '13 at 07:54
0

Here is what I have done to make it work, I could either change parsleyfy class to overwrite the save method like this:

def parsleyfy(klass):
    class ParsleyClass(klass):
        def save(self, *args, **kwargs):
            return super(klass, self).save(*args, **kwargs)
    return ParsleyClass

or change the AccountForm's save method to be like this:

@parsleyfy
class AccountForm(forms.ModelForm):
    def save(self, *args, **kwargs):
        return super(forms.ModelForm, self).save(*args,**kwargs)

One thing I don't what the difference is, is super(Class, self) vs super(Parent, self) I have asked this question

Community
  • 1
  • 1
James Lin
  • 25,028
  • 36
  • 133
  • 233