Re: [Django] #14082: modelform_factory should use the form's metaclass

[Django]

#14082: modelform_factory should use the form's metaclass
-+-
   Reporter:  jspiros|  Owner:  Honza_Kral
   Type:  Bug| Status:  closed
  Milestone: |  Component:  Forms
Version:  SVN|   Severity:  Normal
 Resolution:  fixed  |   Keywords:  ModelForm,
   Triage Stage:  Accepted   |  ModelFormMetaclass,
Needs documentation:  0  |  modelform_factory
Patch needs improvement:  0  |  Has patch:  1
 |Needs tests:  0
 |  Easy pickings:  0
-+-
Changes (by carljm):

 * status:  reopened => closed
 * resolution:   => fixed


Comment:

 In [16334]:
 {{{
 #!CommitTicketReference repository="" revision="16334"
 Fixed #14082 -- Use metaclass of provided ModelForm subclass in
 modelform_factory. Thanks jspiros and Stephen Burrows for the patch.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14082: modelform_factory should use the form's metaclass

[Django]

#14082: modelform_factory should use the form's metaclass
-+-
   Reporter:  jspiros|  Owner:  Honza_Kral
   Type:  Bug| Status:  reopened
  Milestone: |  Component:  Forms
Version:  SVN|   Severity:  Normal
 Resolution: |   Keywords:  ModelForm,
   Triage Stage:  Accepted   |  ModelFormMetaclass,
Needs documentation:  0  |  modelform_factory
Patch needs improvement:  0  |  Has patch:  1
 |Needs tests:  0
 |  Easy pickings:  0
-+-
Changes (by melinath):

 * type:  New feature => Bug
 * needs_tests:  1 => 0
 * easy:   => 0


Comment:

 Updated the patch to r16322 - tests run with no unexpected failures. Added
 tests that fail iff the patch is not applied. Changed type to Bug since an
 erroneous behavior is being corrected.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14082: modelform_factory should use the form's metaclass

[Django]

#14082: modelform_factory should use the form's metaclass
-+-
   Reporter:  jspiros|Owner:  Honza_Kral
 Status:  reopened   |Milestone:
  Component:  Forms  |  Version:  SVN
 Resolution: | Keywords:  ModelForm,
   Triage Stage:  Accepted   |  ModelFormMetaclass,
Needs documentation:  0  |  modelform_factory
Patch needs improvement:  0  |Has patch:  1
 |  Needs tests:  1
-+-
Changes (by melinath):

 * cc: melinath (added)
 * status:  closed => reopened
 * resolution:  worksforme =>


Comment:

 I've looked into this issue and there actually is a problem here.
 Essentially, the !ModelFormMetaclass will be run twice.

 Suppose I define the following modelform subclass:

 {{{
 #!python
 class MyModelFormMetaclass(ModelFormMetaclass):
 run = 0
 def __new__(*args, **kwargs):
 return ModelFormMetaclass.__new__(*args, **kwargs)

 class MyModelForm(ModelForm):
 __metaclass__ = MyModelFormMetaclass
 }}}

 If I use modelform_factory in its current state, the following calls
 occur:

 1. a call to !ModelFormMetaclass.!__new!__
 2. a super() call by !ModelFormMetaclass ![1]
 3. a call to !MyModelFormMetaclass.!__new!__ in the course of the super
 call. This is how type works, apparently.
 4. a call to !ModelFormMetaclass.!__new!__ during
 !MyModelFormMetaclass.!__new!__

 This should be corrected not only because it's inelegant to be calling
 !ModelFormMetaclass.!__new!__ twice and not only because it's odd that the
 code currently doesn't call the form's actual metaclass on principle, but
 also because this can cause real issues.

 Just to pick an example, if I try to modify the base_fields of the form in
 my custom metaclass *after* calling !ModelFormMetaclass myself, my changes
 will be overridden by !ModelFormMetaclass once the stack gets back up to
 it.

 ![1]
 http://code.djangoproject.com/browser/django/trunk/django/forms/models.py#L194

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14082: modelform_factory should use the form's metaclass

[Django]

#14082: modelform_factory should use the form's metaclass
-+--
  Reporter:  jspiros | Owner:  Honza_Kral   
   
Status:  closed  | Milestone:   
   
 Component:  Forms   |   Version:  SVN  
   
Resolution:  worksforme  |  Keywords:  ModelForm, 
ModelFormMetaclass, modelform_factory
 Stage:  Accepted| Has_patch:  1
   
Needs_docs:  0   |   Needs_tests:  1
   
Needs_better_patch:  0   |  
-+--
Changes (by jakub_vysoky):

  * status:  assigned => closed
  * resolution:  => worksforme

Comment:

 I hope I made myself clear, that it does work for me even without the
 patch. But maybe you need something more specific and you can put some
 more tests for that.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14082: modelform_factory should use the form's metaclass

[Django]

#14082: modelform_factory should use the form's metaclass
---+
  Reporter:  jspiros   | Owner:  Honza_Kral 
 
Status:  assigned  | Milestone: 
 
 Component:  Forms |   Version:  SVN
 
Resolution:|  Keywords:  ModelForm, ModelFormMetaclass, 
modelform_factory
 Stage:  Accepted  | Has_patch:  1  
 
Needs_docs:  0 |   Needs_tests:  1  
 
Needs_better_patch:  0 |  
---+
Comment (by jakub_vysoky):

 I've made some tests for this, but it seems it really works.. Can you look
 at it?

 https://github.com/kvbik/django/commit/c342c3089ac169fbcf285b34f65a73fd834886a7

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14082: modelform_factory should use the form's metaclass

[Django]

#14082: modelform_factory should use the form's metaclass
---+
  Reporter:  jspiros   | Owner:  Honza_Kral 
 
Status:  assigned  | Milestone: 
 
 Component:  Forms |   Version:  SVN
 
Resolution:|  Keywords:  ModelForm, ModelFormMetaclass, 
modelform_factory
 Stage:  Accepted  | Has_patch:  1  
 
Needs_docs:  0 |   Needs_tests:  1  
 
Needs_better_patch:  0 |  
---+
Changes (by Honza_Kral):

  * owner:  nobody => Honza_Kral
  * status:  new => assigned
  * stage:  Unreviewed => Accepted

Comment:

 We still need some tests to verify the fix, but the issue is valid

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14082: modelform_factory should use the form's metaclass

[Django]

#14082: modelform_factory should use the form's metaclass
-+--
  Reporter:  jspiros | Owner:  nobody   
   
Status:  new | Milestone:   
   
 Component:  Forms   |   Version:  SVN  
   
Resolution:  |  Keywords:  ModelForm, 
ModelFormMetaclass, modelform_factory
 Stage:  Unreviewed  | Has_patch:  1
   
Needs_docs:  0   |   Needs_tests:  1
   
Needs_better_patch:  0   |  
-+--
Comment (by jspiros):

 Replying to [comment:1 mitar]:
 > I am not sure that this is the case. I play a lot with metaclasses
 [comment:ticket:7018:9 here] and it seems to work?

 Metaclasses work, but only if you directly instantiate the ModelForm
 subclass yourself. modelform_factory dynamically creates a new form class
 with the passed-in class as the parent, and uses ModelFormMetaclass as the
 constructor. So, when using modelform_factory with ModelForm subclasses
 that have a custom metaclass set, that custom metaclass is ignored.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.

Re: [Django] #14082: modelform_factory should use the form's metaclass

[Django]

#14082: modelform_factory should use the form's metaclass
-+--
  Reporter:  jspiros | Owner:  nobody   
   
Status:  new | Milestone:   
   
 Component:  Forms   |   Version:  SVN  
   
Resolution:  |  Keywords:  ModelForm, 
ModelFormMetaclass, modelform_factory
 Stage:  Unreviewed  | Has_patch:  1
   
Needs_docs:  0   |   Needs_tests:  1
   
Needs_better_patch:  0   |  
-+--
Changes (by mitar):

 * cc: mmi...@gmail.com (added)

Comment:

 I am not sure that this is the case. I play a lot with metaclasses
 [comment:ticket:7018:9 here] and it seems to work?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-upda...@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-updates?hl=en.