Opened 12 years ago

Closed 11 years ago

#17447 closed Cleanup/optimization (worksforme)

Problematic logical expressions for types.ClassType checks in django.utils.dictconfig

Reported by: Stefan Zimmermann Owned by: nobody
Component: Python 3 Version: dev
Severity: Normal Keywords: types ClassType logic expression hasattr
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

BaseConfigurator.configure_custom and DictConfigurator.configure_handler both contain the following code:

if not hasattr(c, '__call__') and hasattr(types, 'ClassType') and type(c) != types.ClassType:
    c = self.resolve(c)

The problem is that this logical expression always fails if the types module doesn't have the ClassType attribute (I encountered this when working with the django-3k fork)

The expression could be written like this:

if not hasattr(c, '__call__') and (not hasattr(types, 'ClassType') or type(c) != types.ClassType):
    c = self.resolve(c)

But the hasattr call could also be completely dropped (In python 2.x types.ClassType is always there and 2to3 converts it to type):

if not hasattr(c, '__call__') and type(c) != types.ClassType:
    c = self.resolve(c)

Attachments (1)

django.utils.dictconfig.diff (1005 bytes ) - added by Stefan Zimmermann 12 years ago.

Download all attachments as: .zip

Change History (6)

by Stefan Zimmermann, 12 years ago

comment:1 by Aymeric Augustin, 12 years ago

Component: UncategorizedCore (Other)

Django's dictconfig is a literal copy of the one in Python 2.7, included verbatim to ensure compatibility across Python versions.

Has this issue been reported and/or fixed in Python's native dictconfig?

If there's a problem with the version bundled with Django, the preferred fix is to update to a new verbatim copy, not to patch Django's version.

comment:2 by Łukasz Rekucki, 12 years ago

Triage Stage: UnreviewedAccepted

It a non issue for Python, beacause this code work in Python 2.x, and Python 3.2+ just uses if not callable(c). One way to fix this, without touching the verbatim copy would be to alter 2to3, to fix the check.

I may be missing something, but isn't this check exactly equal to what callable() does? If yes, I'm in favor of changing this.

PS. I wonder what's the story behind this subtle diffrence in what callable() does:

2.x: http://hg.python.org/cpython/file/7c263bfc92f5/Objects/object.c#l1650

3.2: http://hg.python.org/cpython/file/cd748bab3cdd/Objects/object.c#l1179

Version 0, edited 12 years ago by Łukasz Rekucki (next)

comment:3 by Łukasz Rekucki, 12 years ago

Patch needs improvement: set

comment:4 by Aymeric Augustin, 11 years ago

Component: Core (Other)Utilities

comment:5 by Aymeric Augustin, 11 years ago

Component: UtilitiesPython 3
Resolution: worksforme
Status: newclosed

This bug was filed against an old experimental port to Python 3 that was abandonned.

Django now attemps to import dictConfig from logging.config, and falls back to importing it from django.utils.dictconfig only on Python 2.6.

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