Opened 8 years ago

Closed 8 years ago

#26311 closed Uncategorized (invalid)

Clean django.contrib.admin.widgets.RelatedFieldWidgetWrapper.__deepcopy__

Reported by: James Pic Owned by: nobody
Component: Uncategorized Version: 1.9
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently, django.contrib.admin.widget.RelatedFieldWidgetWrapper defines:

    def __deepcopy__(self, memo):
        obj = copy.copy(self)
        obj.widget = copy.deepcopy(self.widget, memo)
        obj.attrs = self.widget.attrs
        memo[id(self)] = obj
        return obj

I tried to run the tests (with python3 runtests.py admin*) after removing that, and it seemed like it didn't break anything. So my first question is: what use case is this meant to support ? Is that use case tested ?

Also, RelatedFieldWidgetWrapper inherits from Widget, which defines:

    def __deepcopy__(self, memo):
        obj = copy.copy(self)
        obj.attrs = self.attrs.copy()
        memo[id(self)] = obj
        return obj

Question is: can we change RelatedFieldWidgetWrapper.deepcopy to:

    def __deepcopy__(self, memo):
        obj = super(RelatedFieldWidgetWrapper, self).__deepcopy__(memo)
copy.copy(self)
        obj.widget = copy.deepcopy(self.widget, memo)
        return obj

Like in MultiWidgets, which has:

    def __deepcopy__(self, memo):
        obj = super(MultiWidget, self).__deepcopy__(memo)
        obj.widgets = copy.deepcopy(self.widgets)
        return obj

Another problem I found is that it didn't seem to break any test to just remove RelatedFieldWidgetWrapper.deepcopy. Is it tested anywhere ? Or should we add a test for that too ?

Change History (1)

comment:1 by James Pic, 8 years ago

Resolution: invalid
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top