Opened 2 years ago

Closed 2 years ago

#19543 closed Bug (fixed)

SimpleLazyObject missing __repr__ proxy

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

Description

SimpleLazyObject (from django.utils.functionals) does not proxy _repr_ method.
Use Case:

User.__unicode__ -> print data for user, example: "user@email.com"
User.__repr__ -> print debugging data, example: "<User pk:1, email:user@email.com, points:123>"

_unicode_ is used by most of application to render user content so I cant override it to using at logging. _repr_ is used in logging.

Example user message: "Hi %s"%request.user
Example logging message: "%r just logged in"%request.user

When user is wrapped with SimpleLazyObject _repr_ method of User is not called.
My idea is to make it available like

SimpleLazyObject._repr_ = lambda self: '<SimpleLazyObject: %r>'%self._wrapped

Patch:

--- a/django/utils/functional.py
+++ b/django/utils/functional.py
@@ -303,6 +303,9 @@ class SimpleLazyObject(LazyObject):
     def __reduce__(self):
         return (self.__newobj__, (self.__class__,), self.__getstate__())
 
+    def __repr__(self):
+        return '<SimpleLazyObject: %r>' % self._wrapped
+
     # Need to pretend to be the wrapped class, for the sake of objects that care
     # about this (especially in equality tests)
     __class__ = property(new_method_proxy(operator.attrgetter("__class__")))

Attachments (1)

implemented_repr.diff (2.0 KB) - added by fhahn 2 years ago.
implemented repr and updated test

Download all attachments as: .zip

Change History (7)

comment:1 Changed 2 years ago by ptone

  • Component changed from Uncategorized to Core (Other)
  • Easy pickings set
  • Needs documentation unset
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.5-beta-1 to master

Since a lazy object represents itself as the target class - it should try to be that class as much as possible, and not try to be "something special" even though it is.

So repr should just proxy like other magic methods on the class.

Accepting the ticket based on the fact that we should do something explicit for repr, but rejecting the wrapped result.

Version 0, edited 2 years ago by ptone (next)

comment:2 Changed 2 years ago by ptone

After some further discussion about the value in debugging the following is now suggested:

Do show in __repr__ that we are a lazy object

check whether the lazy object has been evaluated

if not evaluated, include the repr of the setup function as the wrapped content - so that lazy objects are not evaluated at the time of repr call

otherwise wrap the target instances __repr__ method as proposed.

Changed 2 years ago by fhahn

implemented repr and updated test

comment:3 Changed 2 years ago by fhahn

  • Cc flo@… added
  • Needs tests unset
  • Owner changed from nobody to fhahn
  • Patch needs improvement unset
  • Status changed from new to assigned

comment:4 Changed 2 years ago by oinopion

  • Triage Stage changed from Accepted to Ready for checkin

Looks good!

comment:5 Changed 2 years ago by fhahn

forgot to link the github pull request: https://github.com/django/django/pull/709

comment:6 Changed 2 years ago by Preston Holmes <preston@…>

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

In 0ea5bf88dddd7fbdef7fe8d00162c9753565f5c0:

Fixed #19543 -- implemented SimpleLazyObject.repr

Thanks to Florian Hahn for the patch

Note: See TracTickets for help on using tickets.
Back to Top