Django

Code

Ticket #12048 (closed: fixed)

Opened 10 months ago

Last modified 5 months ago

MultiWidget does not define __deepcopy__

Reported by: powderflask <powderflask@gmail.com> Assigned to: nobody
Milestone: 1.2 Component: Forms
Version: SVN Keywords:
Cc: ben@aashe.org Triage Stage: Design decision needed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

django.forms.MultiWidget defines an instance variable, widgets , which is a list of widgets. However, it does not override the deepcopy() method in the Widget base class to make a copy of this instance variable. A deepcopy of the widgets is needed when an instance when a django Form containing a Field that uses a MultiWidget is instantiated, otherwise all such forms have a reference to the same widgets list rather than their own copy.

The patch is simple - override deepcopy() in MultiWidget to make a copy of widgets. A proposed patch is attached.

Replicating the problem requires that we extend MultiWidget, make a deepcopy of the widget, then alter one of the widgets in some way. Here is a minimal example that demonstrates the issue - applying the patch fixes the issue.

from django import forms
from django.utils.encoding import StrAndUnicode, force_unicode
from django.utils.safestring import mark_safe

#####################################################
## Simplified ChoiceWithOtherWidget  
##   original downloaded from: http://www.djangosnippets.org/snippets/863/
#####################################################
class ChoiceWithOtherWidget(forms.MultiWidget):
    """MultiWidget for use with ChoiceWithOtherField."""
    def __init__(self, choices=[]):
        widgets = [
            forms.RadioSelect(choices=choices),
            forms.TextInput
        ]
        super(ChoiceWithOtherWidget, self).__init__(widgets)
    
    def _set_choices(self, choices):
        """ When choices are set for this widget, we want to pass those along to the Select widget"""
        self.widgets[0].choices = choices
    def _get_choices(self):
        """ The choices for this widget are the Select widget's choices"""
        return self.widgets[0].choices
    choices = property(_get_choices, _set_choices)
    
    def decompress(self, value):
        if not value:
            return [None, None]
        return value


#################################################
##  Minimal code to reproduces bug
#################################################
from copy import deepcopy

widget1 = ChoiceWithOtherWidget(choices=['A','B','C'])
widget2 = deepcopy(widget1)
widget2.choices=['X','Y','Z']

widget1.choices
widget2.choices

Here is the output from running the sample code above in a shell before the patch was applied:

>>> widget1 = ChoiceWithOtherWidget(choices=['A','B','C'])
>>> widget2 = deepcopy(widget1)
>>> widget2.choices=['X','Y','Z']
>>> 
>>> widget1.choices
['X', 'Y', 'Z']
>>> widget2.choices
['X', 'Y', 'Z']

... and after the patch was applied:

>>> widget1 = ChoiceWithOtherWidget(choices=['A','B','C'])
>>> widget2 = deepcopy(widget1)
>>> widget2.choices=['X','Y','Z']
>>> 
>>> widget1.choices
['A', 'B', 'C']
>>> widget2.choices
['X', 'Y', 'Z']

Attachments

widgets.py.patch (0.6 kB) - added by powderflask <powderflask@gmail.com> on 10/17/09 14:02:08.
patch for MultiWidget? deepcopy bug (django.forms.MultiWidget?)

Change History

10/17/09 14:02:08 changed by powderflask <powderflask@gmail.com>

  • attachment widgets.py.patch added.

patch for MultiWidget? deepcopy bug (django.forms.MultiWidget?)

10/18/09 19:46:35 changed by jamstooks

  • cc set to ben@aashe.org.
  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

12/15/09 10:09:29 changed by jcd

  • stage changed from Unreviewed to Design decision needed.

03/09/10 16:52:04 changed by lukeplant

  • status changed from new to closed.
  • resolution set to fixed.

(In [12739]) Fixed #12048 - MultiWidget? does not define __deepcopy__

Thanks to powderflask for report, test case and initial patch

03/09/10 16:59:04 changed by lukeplant

(In [12740]) [1.1.X] Fixed #12048 - MultiWidget? does not define __deepcopy__ Thanks to powderflask for report, test case and initial patch.

Backport of [12739] from trunk


Add/Change #12048 (MultiWidget does not define __deepcopy__)




Change Properties
Action