Opened 14 years ago
Closed 13 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)
Change History (6)
by , 14 years ago
| Attachment: | django.utils.dictconfig.diff added |
|---|
comment:1 by , 14 years ago
| Component: | Uncategorized → Core (Other) |
|---|
comment:2 by , 14 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
It's 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
comment:3 by , 14 years ago
| Patch needs improvement: | set |
|---|
comment:4 by , 13 years ago
| Component: | Core (Other) → Utilities |
|---|
comment:5 by , 13 years ago
| Component: | Utilities → Python 3 |
|---|---|
| Resolution: | → worksforme |
| Status: | new → closed |
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.
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.