#21733 closed Bug (invalid)
@python_2_unicode_compatible causes infinite recursion when module imported more than once
Reported by: | ntucker | Owned by: | nobody |
---|---|---|---|
Component: | Utilities | Version: | 1.6 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Sometimes a module that uses @python_2_unicode_compatible is imported multiple times. Possibly because a different pathname is used to import it. When this occurs, it makes unicode be the str function that was originally assigned, which calls unicode. So anytime unicode is called on the object, there is infinite recursion.
A simple check whether the str function has already been assigned will prevent setting unicode to the provided recursive function. This will also guard against other cases of multiple @python_2_unicode_compatible calls.
class _STR(object): def __call__(self, obj): return self.__unicode__().encode('utf-8') _str = _STR() def python_2_unicode_compatible(klass): """ A decorator that defines __unicode__ and __str__ methods under Python 2. Under Python 3 it does nothing. To support Python 2 and 3 with a single code base, define a __str__ method returning text and apply this decorator to the class. """ if six.PY2: if '__str__' not in klass.__dict__: raise ValueError("@python_2_unicode_compatible cannot be applied " "to %s because it doesn't define __str__()." % klass.__name__) # this was already applied once if klass.__str__ == _str: return klass klass.__unicode__ = klass.__str__ klass.__str__ = _str return klass
I will provide patch if this is accepted. Above is example code.
Attachments (2)
Change History (10)
by , 11 years ago
Attachment: | recursionpatch.diff added |
---|
comment:1 by , 11 years ago
Has patch: | set |
---|
comment:3 by , 11 years ago
Easy pickings: | unset |
---|---|
Keywords: | python2.7.3 removed |
tl;dr
Don't import modules twice.
Django doesn't require putting a directory and a subdirectory on PYTHONPATH since 1.4 and will most likely actively prevent it (i.e. refuse to start) in future versions.
explanation
I assume the problem only happens on Django model classes. The root cause isn't obvious and doesn't lie in python_2_unicode_compatible.
The second import should create a new class and apply the proper transformations to its methods. Unfortunately, the devious ModelBase.__new__
metaclass gets in the way and returns the existing class, to which the decorator has already been applied. So it gets applied a second time and that breaks.
As part of the app-loading refactor, I plan to forbid importing models twice to remove that, err, interesting behavior. This is tracked in #21711 and will make it impossible to trigger this issue.
That said, mistakes such as applying the decorator to a subclass that doesn't override __str__
also trigger an infinite recursion. I'd like to raise a specific exception when the decorator is applied twice to the same methods (ie. twice to the same class, or to a class and a subclass when no subclass has overridden __str__
in the mean time).
The appropriate way to achive this is to set an attribute on the __unicode__
method when it has been created by the decorator, and to test for that attribute and raise an exception if it's set.
comment:4 by , 11 years ago
Hmm, I'm getting multiple imports because I do from .models import Whatever in the application, then at other places from application.models import whatever.
I also put my models in their own modules, and package them up so init.py has from .whatever import *
I don't see any way of avoiding these different import styles
As far as setting an attribute on str; I tried this and it would not allow it saying
AttributeError: 'instancemethod' object has no attribute '__secrettest'
when I try to
setattr(klass.__str__, '__secrettest', True)
comment:5 by , 11 years ago
Regardless of your import style, double imports can happen only if you have a directory and a subdirectory on PYTHONPATH.
Read this explanation by a Python core dev if you want to learn more on this topic: http://python-notes.curiousefficiency.org/en/latest/python_concepts/import_traps.html#the-double-import-trap
comment:6 by , 11 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
I had written a patch, but then I realized that the current code correctly handles the subclass case.
The patch is only useful when metaclass magic messes things up. I'm attaching it for the record, but I now believe that #21711 is the only action to take in response to this bug.
by , 11 years ago
Attachment: | 21733.patch added |
---|
comment:7 by , 11 years ago
Thanks for the explanation. For some reason all the other places I found did not explain module importing properly. :)
comment:8 by , 10 years ago
Thanks for tracking down the cause of this! I wanted to stop the double imports from happening and be able to use relative imports where possible, so I did some more digging and found this SO post:
http://stackoverflow.com/questions/23527435/python-path-not-consistent-in-django-double-module-imports
I still had an __init__.py
in my project root directory -- deleting that file stopped the double imports and thus stopped the infinite recursion for me.
Patch