Code

Opened 3 years ago

Closed 21 months ago

#15811 closed Bug (fixed)

Lazy doesn't take into account methods defined in parents

Reported by: abki Owned by: nobody
Component: Core (Other) Version: master
Severity: Normal Keywords:
Cc: abki Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Example:

from django.utils.datastructures import SortedDict
from django.utils.functional import lazy

def a(): return SortedDict([(2,2),(1,1)])

f = lazy(a, SortedDict)()
f[1]

This results in a TypeError

I got a patch ready but not sure if it does what it should do well since I don't grasp all the lazy functionality.

Attachments (4)

deep_lazy.diff (1.5 KB) - added by abki 3 years ago.
patch that deeply "copy" a class methods
deep_copy.patch (2.3 KB) - added by abki 3 years ago.
added a test
lazy_base_class_aware.diff (2.3 KB) - added by abki 3 years ago.
lastest version of the patch :)
lazy.with_dir.patch (1.5 KB) - added by abki 2 years ago.
git format

Download all attachments as: .zip

Change History (12)

Changed 3 years ago by abki

patch that deeply "copy" a class methods

comment:1 Changed 3 years ago by abki

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Version changed from 1.2 to SVN

comment:2 Changed 3 years ago by abki

  • Has patch set
  • Type changed from Uncategorized to Bug

Changed 3 years ago by abki

added a test

comment:3 Changed 3 years ago by lukeplant

  • Component changed from Uncategorized to Core (Other)
  • Easy pickings unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

'deep copy' is confusing terminology - it normally refers to copy.deepcopy. What you are referring to is checking base class methods. The test name and comments need to be updated accordingly.

In addition, I think that test should not implicitly rely on the fact that SortedDict doesn't have __getitem__ defined - that could easily change. I would define a pair of custom classes that show the problem, probably within the test method itself.

Otherwise, this looks pretty good.

comment:4 Changed 3 years ago by abki

  • Cc abki added

changed the test & added proper documentation

Changed 3 years ago by abki

lastest version of the patch :)

comment:5 Changed 3 years ago by lukeplant

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

Thanks!

For future reference, when you have updated a patch, you should uncheck the 'Patch needs improvement' flag if you think the patch is up to scratch. A reviewer will set it again if it isn't.

comment:6 Changed 3 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from new to closed

In [16157]:

Fixed #15811 - lazy() doesn't take into account methods defined in parents

Thanks to abki for the report and patch.

Changed 2 years ago by abki

git format

comment:7 Changed 2 years ago by abki

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • UI/UX unset

new patch which avoids using double underscores.

comment:8 Changed 21 months ago by lukeplant

  • Resolution set to fixed
  • Status changed from reopened to closed

It's not at all obvious to me that this new patch does the same as the existing code, especially given the rather vague definition of 'dir': "return an alphabetized list of names comprising (some of) the attributes of the given object, and of attributes reachable from it."

So I'm closing again. Since the original bug is fixed, please open a new ticket or pull request for any cleanup.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.