Code

Opened 6 years ago

Closed 2 years ago

#6897 closed Cleanup/optimization (worksforme)

Convert more old-style classes to new-style classes

Reported by: brodie Owned by: nobody
Component: Uncategorized Version: master
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 6 years ago.
Patch replacing old-style classes with new-style classes
newstyledocs.diff (1.2 KB) - added by brodie 6 years ago.
Old-style/new-style updates in the docs

Download all attachments as: .zip

Change History (9)

Changed 6 years ago by brodie

Patch replacing old-style classes with new-style classes

Changed 6 years ago by brodie

Old-style/new-style updates in the docs

comment:1 follow-up: Changed 6 years ago by axiak

  • Needs documentation unset
  • Needs tests unset
  • 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?

comment:2 in reply to: ↑ 1 ; follow-up: Changed 6 years ago by brodie

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.

comment:3 in reply to: ↑ 2 ; follow-up: Changed 6 years ago by 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.

comment:4 Changed 6 years ago by axiak

  • Triage Stage changed from Unreviewed to Accepted

Oh, and I think we should accept this ticket.

comment:5 in reply to: ↑ 3 Changed 6 years ago by brodie

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 Changed 3 years ago by baumer1122

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:7 Changed 2 years ago by adamnelson

  • Easy pickings unset
  • Resolution set to worksforme
  • Status changed from new to closed
  • 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)

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.