Better newforms metaclassing

[Chris Beaven]

I had a closer look at the metaclassing of newforms. Seems like BaseForm could benefit from a little metaclass magic so that you can do multiple subclasses of BaseForms created with form_for_*

[simo <simon@…>]

Yes!!!! I was up all last night, banging my head against a brick wall trying to figure out why form_for_* didn't want to play with the cunning hierarchy of forms I'd hashed together. Nice work Chris!

[simo <simon@…>]

Mix-in question. When I call save(), which one does it call? The last one? Or does it call all of them in succession?

I guess we might need more magic if it only manages to call the last one...

[James Bennett]

I'd say this is something to bring up on the dev list; personally, I don't like the idea of subclassing the result of form_for_model or form_for_instance, because those utilities are -- as I understand it -- intended for the use case where you want the default form, period, and that those who want customized behavior should be writing custom form classes instead of poking around in auto-generated convenience classes.

[simo <simon@…>]

After much pain and suffering, I'm beginning to agree with ubernostrum. Be good to give some guidelines like this in the docs.

[Chris Beaven]

Replying to simo <simon@quo.com.au>:

Mix-in question. When I call save(), which one does it call? The last one? Or does it call all of them in succession?

It calls the first one.

From Python docs:

class !DerivedClassName(Base1, Base2, Base3):
    ...

The only rule necessary to explain the semantics is the resolution rule used for class attribute references. This is depth-first, left-to-right. Thus, if an attribute is not found in DerivedClassName, it is searched in Base1, then (recursively) in the base classes of Base1, and only if it is not found there, it is searched in Base2, and so on.

So yes, you'd need to make a custom save method (which probably returns a tuple of the subclassed save methods.

It's not really the responsibility of this patch to fix those sort of things for you. I just wanted BaseForm to be subclassable so that the base_fields are shared like they are with !Form - that's all it is supposed to achieve.

[simo <simon@…>]

Thanks Chris. To be honest though I'm a bit disillusioned by things that imply they work but don't fully (to a lay at least). I mean, as a user, if you mix two forms together, you would expect them to incorporate both in terms of functionality. Magic all the way, or if that's too hard then none at all I'd say. And either way it needs to be covered in docs too.

[Chris Beaven]

The goal of this patch is to move some of the metaclass magic to BaseForm so it can inherit base_fields from any superclasses. Sorry for any implications further than this, I wasn't very clear with the initial description.

It should be a totally backwards compatible change (passes all tests and has been working for me) and it's more of an internal change than something that needs documentation. I don't think the average user *should* be trying to use multiple inheritance - but I found it useful for a mix-in class recently.

[Chris Beaven]

Ticket #3815 was reopened because my solution (which worked then) now doesn't - this patch makes it work again.

[Chris Beaven]

#4239 (closed as a dupe of this one) shows the problem I'm solving here...

[Malcolm Tredinnick]

This needs some django-dev discussion. I think the test case is not necessarily a good goal because a lot of people are going to try to force it to behave like Simon was expecting (the test looks like it's a combination of two fields, even though if you remember it's Python you realise it's just multiple inheritance).

I also feel a bit suspicious about the design. Both Form and BaseForm end up with the same root metaclass (because DeclarativeFieldsMetaclass inherits from BaseFieldsMetaclass). I don't have a concrete reason why I don't like that yet, but it smells funny. I can appreciate that mixins to introduce common fields would be nice, but the implementation is fiddly. If the goal is to shoehorn more tricks into form_for_*, I'm going to be a hard sell.

[Chris Beaven]

There could be some messy side effects of this. I noticed it was changing the class of forms under certain circumstances. I'll close for now.