Opened 16 years ago

Closed 12 years ago

#6897 closed Cleanup/optimization (worksforme)

Convert more old-style classes to new-style classes

Reported by: Brodie Rao Owned by: nobody
Component: Uncategorized Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I see long ago, in #2109, someone went through and replaced all usage of old-style classes with new-style classes. It seems there are still old-style classes being defined, so I've gone through and updated those as well.

One thing to note: db.backends.mysql_old.base.MysqlDebugWrapper has __getattr__ defined, but the function assumes that __getattr__ is called for attributes that already exist, which isn't the case. I've removed the code that looks up existing attributes, since it would never be executed in the first place.

There are also some old-style classes in the copied-over test._doctest module, which I didn't touch.

Attachments (2)

newstyle.diff (9.0 KB ) - added by Brodie Rao 16 years ago.
Patch replacing old-style classes with new-style classes
newstyledocs.diff (1.2 KB ) - added by Brodie Rao 16 years ago.
Old-style/new-style updates in the docs

Download all attachments as: .zip

Change History (9)

by Brodie Rao, 16 years ago

Attachment: newstyle.diff added

Patch replacing old-style classes with new-style classes

by Brodie Rao, 16 years ago

Attachment: newstyledocs.diff added

Old-style/new-style updates in the docs

comment:1 by Michael Axiak, 16 years ago

Patch needs improvement: set

Hi brodie,

Two comments with your patch:

  1. Your new implementation of getattr... doesn't that cause infinite recursion?
  2. Why did you move pass to a newline in one instance bot not on the other?

in reply to:  1 ; comment:2 by Brodie Rao, 16 years ago

Replying to axiak:

  1. Your new implementation of getattr... doesn't that cause infinite recursion?

I haven't changed that part of the implementation; it's getattr(self.cursor, attr), not getattr(self, attr).

  1. Why did you move pass to a newline in one instance bot not on the other?

The one instance where I didn't do that was in some example code in a doc string. I didn't really give it that much thought, but I guess it could be changed there as well.

in reply to:  2 ; comment:3 by Michael Axiak, 16 years ago

Replying to brodie:

I haven't changed that part of the implementation; it's getattr(self.cursor, attr), not getattr(self, attr).

Yes. And what will self.cursor do in that statement? You should try running this for yourself.

comment:4 by Michael Axiak, 16 years ago

Triage Stage: UnreviewedAccepted

Oh, and I think we should accept this ticket.

in reply to:  3 comment:5 by Brodie Rao, 16 years ago

Replying to axiak:

Replying to brodie:

I haven't changed that part of the implementation; it's getattr(self.cursor, attr), not getattr(self, attr).

Yes. And what will self.cursor do in that statement? You should try running this for yourself.

As I explained, __getattr__ is called only for attributes that don't already exist on the object. Since cursor already exists on the object, it's not called, and there's no infinite recursion. As the official documentation states: "__getattr__(self, name) Called when an attribute lookup has not found the attribute in the usual places (i.e. it is not an instance attribute nor is it found in the class tree for self)."

Let me demonstrate:

>>> class Mock(object): pass
... 
>>> cursor = Mock()
>>> cursor.foo = 5
>>> w = MysqlDebugWrapper(cursor)
>>> w.cursor # This is already defined on w, so __getattr__ isn't called
<__main__.Mock object at 0x77ab0>
>>> w.foo # This is not defined on w, so __getattr__ IS called, which in turn accesses w.cursor, which in turn doesn't call __getattr__
5

comment:6 by Peter Baumgartner, 13 years ago

Severity: Normal
Type: Cleanup/optimization

comment:7 by Adam Nelson, 12 years ago

Easy pickings: unset
Resolution: worksforme
Status: newclosed
UI/UX: unset

This stuff was all fixed long ago. There are no old classes AFAIK and the ones that were patched in this ticket are fixed (i.e. https://code.djangoproject.com/browser/django/trunk/django/db/backends/sqlite3/introspection.py)

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