classification
Title: Ignore missing attributes in functools.update_wrapper
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.2
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: Antoine d'Otreppe, BreamoreBoy, Yaniv.Aknin, benjamin.peterson, brian.curtin, eklitzke, eric.araujo, findepi, freakboy3742, july, ncoghlan, pitrou, yaubi
Priority: normal Keywords: needs review, patch

Created on 2008-07-25 15:26 by Antoine d'Otreppe, last changed 2012-02-26 02:58 by eric.araujo. This issue is now closed.

Files
File name Uploaded Description Edit
fix.patch eklitzke, 2010-01-14 00:50 trivial change to ignore missing "asssigned" attributes
fix.patch eklitzke, 2010-01-16 04:11 updated patch
update_wrapper-ignore-missing-attributes.patch july, 2010-05-02 14:06 patch with test updated (apply to trunk)
Messages (20)
msg70256 - (view) Author: Antoine d'Otreppe (Antoine d'Otreppe) Date: 2008-07-25 15:26
When trying to do something like
"functools.update_wrapper(myWrapper, str.split)"

I got this error message:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "Aspyct.py", line 175, in beforeCall
    _stickAdvice(function, theAdvice)
  File "Aspyct.py", line 90, in _stickAdvice
    functools.update_wrapper(theAdvice, function)
  File "/usr/lib/python2.5/functools.py", line 33, in update_wrapper
    setattr(wrapper, attr, getattr(wrapped, attr))
AttributeError: 'method_descriptor' object has no attribute '__module__'
msg70320 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-07-27 13:05
I think what you want is

functools.update_wrapper(myWrapper, str.split, ('__name__', '__doc__'))
msg70328 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2008-07-27 18:12
The problem actually has to do with trying to use update_wrapper on a
method instead of a function - bound and unbound methods don't have all
the same attributes that actual functions do.

>>> import functools
>>> functools.WRAPPER_ASSIGNMENTS
('__module__', '__name__', '__doc__')
>>> functools.WRAPPER_UPDATES
('__dict__',)
>>> def f(): pass
...
>>> class C:
...   def m(): pass
...
>>> set(dir(f)) - set(dir(C.m))
set(['func_closure', 'func_dict', '__module__', 'func_name',
'func_defaults', '__dict__', '__name__', 'func_code', 'func_doc',
'func_globals'])

Is an exception the right response when encountering a missing
attribute? Given that the error can be explicitly silenced by writing
"functools.update_wrapper(g, str.split,
set(functools.WRAPPER_ASSIGNMENTS) & set(dir(str.split)))", I'm inclined
to think the current behaviour is the correct option.

Since this is an issue that doesn't come up with the main intended use
case for update_wrapper (writing decorators), and since it can be
handled easily by limiting the set of attributes copied to those the
object actually has, I'm converting this tracker item to an enhancement
request asking for a shorter way of spelling "ignore missing attributes"
(e.g. a keyword-only flag).
msg70331 - (view) Author: Antoine d'Otreppe (Antoine d'Otreppe) Date: 2008-07-28 00:05
Thank you for considering this report :)
msg70352 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2008-07-28 15:29
Another possibility would be for methods to also get the __name__ and
__module__ attributes. I don't see any counter-indication to it.
msg71677 - (view) Author: Piotr Findeisen (findepi) Date: 2008-08-21 20:33
IMO, I'd be better to always ignore missing attributes. Consider another
use case (following code I've posted also on comp.lang.python):

from functools import wraps, partial
def never_throw(f):
    @wraps(f)
    def wrapper(*args, **kwargs):
        try: return f(*args, **kwargs)
        except: pass
    return wrapper 

Looks reasonable. But if I write
never_throw(partial(int))('45a')
I got the exception saying partial objects also don't have __module__
attribute.

I was to use some additional parameters to @wraps, I would have to
rewrite all my decorator definitions. And stil -- the main intent of
wraps and update_wrapper is to write decorators, so IMO they should not
throw on missing attributes.
msg71695 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2008-08-21 22:30
If the object being wrapped isn't a plain vanilla function, then the
odds are *very* good that __doc__ cannot be copied to the new function
and still be correct. This is definitely the case for both modules and
partial objects.

So for both the use cases mentioned thus far, the AttributeError appears
to me to be flagging a real problem with the way update_wrapper is being
used.
msg97745 - (view) Author: Evan Klitzke (eklitzke) Date: 2010-01-14 00:50
I'm also interested in seeing this fixed. In the current behavior, the following code doesn't work:

<<< start code
from functools import wraps

def magic(func):
	@wraps(func)
	def even_more_magic(*args):
		return func(*args)
	return even_more_magic

class Frob(object):
	@magic
	@classmethod
	def hello(cls):
		print '%r says hello' % (cls,)
>>> end code

It fails because classmethods don't have a __module__ attribute, as commented upon elsewhere in this issue. To fix this, you'd either have to either pass in an `assigned` parameter to the `wraps` function, or swap the order of decorator application (i.e. `classmethod(magic(hello))`).

This seems arbitrary and unnecessarily complex; skipping over a missing __module__ should be just fine. Mixing `functools.wraps` and `classmethod` is a relatively common use case that should be as simple as possible.

I've attached a trivial patch which just ignores missing "assigned" attributes.
msg97829 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-01-15 18:11
The patch should come with an unit test (in Lib/test/test_functools.py).
msg97862 - (view) Author: Evan Klitzke (eklitzke) Date: 2010-01-16 04:11
New patch included, with a test case.

I had wanted to check the classmethod __module__ thing directly, but that proved to be elusive, since the classmethod gets the __module__ attribute if the module is '__main__', and you can't delete that attribute. My test just tries to assign another attribute which doesn't exist.

I just tried to copy the style of the rest of the module, lmk if there are any problems.
msg97908 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-01-16 21:39
In your test, the more common convention is to use assertFalse(foo) instead of assert_(not foo).
msg97909 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-01-16 21:42
I think it would be better to test with some of the real world examples given in this issue: str.split, and a functools.partial object.
msg100933 - (view) Author: Russell Keith-Magee (freakboy3742) Date: 2010-03-12 13:00
As an extra data point: we just hit this problem in Django ticket #13093 (http://code.djangoproject.com/ticket/13093). In our case, a decorator was using wraps(); however, that decorator was breaking when it was used on a class with a __call__ method, because the instance of the class doesn't have a __name__ attribute. 

We've implemented the proposed workaround (i.e., check the attributes that are available and provide that tuple as the assigned argument), but I don't agree that this should be expected behavior. wraps() is used to make a decorated callable look like the callable that is being decorated; if there are different types of callable objects, I would personally expect wraps() to adapt to the differences, not raise an error if it sees anything other than a function. 

True, some attributes (like __doc__) won't always be correct as a result of wrapping on non-vanilla functions -- but then, that's true of plain vanilla functions, too. A decorator wrapping a function can fundamentally change what the wrapped function does, and there's no guarantee that the docstring for the wrapped function will still be correct after decoration.
msg104787 - (view) Author: July Tikhonov (july) * Date: 2010-05-02 14:06
Patch updated: bound and unbound methods, user-defined callable, partial object included in test.

By the way,

>>> [id(abs.__doc__) for i in range(5)]
[140714383081744, 140714383081744, 140714383081744, 140714383081744, 140714383081744]
>>> [id(s) for s in [abs.__doc__ for i in range(5)]]
[140714383084040, 140714383082976, 140714383083144, 140714383075904, 140714383081744]

How it can be explained? Built-in functions (and methods) _sometimes_ return a new instance of its '__doc__' (and '__name__'), and sometimes does not.
(I found this trying to include built-in method into the test.)
msg104832 - (view) Author: July Tikhonov (july) * Date: 2010-05-03 12:43
To Evan Klitzke (eklitzke):

>I'm also interested in seeing this fixed. In the current behavior,
>the following code doesn't work:
>
><<< start code
>from functools import wraps
>
>def magic(func):
>	@wraps(func)
>	def even_more_magic(*args):
>		return func(*args)
>	return even_more_magic
>
>class Frob(object):
>	@magic
>	@classmethod
>	def hello(cls):
>		print '%r says hello' % (cls,)
>>>> end code

This code _should not_ work.

[Unbound] classmethod object is not a method or a function, it is even not a callable; it is a descriptor (returning callable). So, it cannot be wrapped or decorated in such way.

If you want something like this to work, you probably should place @classmethod on the upper level (in other words, apply classmethod after all other decorators):

	@classmethod
	@magic
	def hello(cls):
		print '%r says hello' % (cls,)
msg110679 - (view) Author: Mark Lawrence (BreamoreBoy) Date: 2010-07-18 19:47
Is there anyone who can provide this issue with a bit of TLC as it's almost the 2nd birthday?
msg110691 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-07-18 21:39
That would be me :)

In line with the 'consenting adults' philosophy and with the current behaviour causing real world problems, I'll accept this RFE and check it in soon.
msg114101 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-08-17 06:20
Implemented in r84132 (not based on this patch though).
msg154271 - (view) Author: Yaniv Aknin (Yaniv.Aknin) Date: 2012-02-25 16:46
Shouldn't this be fixed in 2.7 as well?

It's a bug, it's in the wild, and it's causing people to do ugly (and maybe not 100% reliable) things like https://code.djangoproject.com/browser/django/trunk/django/utils/decorators.py#L68
msg154301 - (view) Author: √Čric Araujo (eric.araujo) * (Python committer) Date: 2012-02-26 02:58
Well, Nick judged that this was not a bug per se, but rather a request for enhancement, thus it was only committed to 3.3.
History
Date User Action Args
2012-02-26 02:58:53eric.araujosetmessages: + msg154301
2012-02-25 16:50:07eric.araujosetnosy: + eric.araujo
2012-02-25 16:46:58Yaniv.Akninsetnosy: + Yaniv.Aknin
messages: + msg154271
2010-08-17 06:20:13ncoghlansetstatus: open -> closed

messages: + msg114101
stage: patch review -> resolved
2010-07-18 21:39:10ncoghlansetassignee: ncoghlan
resolution: accepted
messages: + msg110691
2010-07-18 19:47:03BreamoreBoysetnosy: + BreamoreBoy

messages: + msg110679
versions: + Python 3.2, - Python 2.6, Python 2.5, Python 3.1, Python 2.7
2010-05-03 12:43:39julysetmessages: + msg104832
2010-05-02 14:06:34julysetfiles: + update_wrapper-ignore-missing-attributes.patch
nosy: + july
messages: + msg104787

2010-03-31 09:21:49yaubisetnosy: + yaubi
2010-03-12 13:00:32freakboy3742setnosy: + freakboy3742

messages: + msg100933
versions: + Python 2.6, Python 2.5
2010-01-16 21:42:55pitrousetmessages: + msg97909
2010-01-16 21:39:38brian.curtinsetnosy: + brian.curtin
messages: + msg97908

keywords: + needs review
stage: patch review
2010-01-16 04:11:30eklitzkesetfiles: + fix.patch

messages: + msg97862
2010-01-15 18:11:52pitrousetmessages: + msg97829
2010-01-14 00:50:40eklitzkesetfiles: + fix.patch

nosy: + eklitzke
messages: + msg97745

keywords: + patch
2008-08-21 22:30:44ncoghlansetmessages: + msg71695
2008-08-21 20:33:40findepisetnosy: + findepi
messages: + msg71677
2008-07-28 15:29:58pitrousetnosy: + pitrou
messages: + msg70352
2008-07-28 00:05:32Antoine d'Otreppesetmessages: + msg70331
2008-07-27 18:13:32ncoghlansettitle: functools.update_wrapper bug -> Ignore missing attributes in functools.update_wrapper
2008-07-27 18:12:08ncoghlansetpriority: normal
assignee: ncoghlan -> (no value)
type: behavior -> enhancement
messages: + msg70328
versions: + Python 3.1, Python 2.7, - Python 2.5
2008-07-27 13:05:11benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg70320
2008-07-27 07:47:31georg.brandlsetassignee: ncoghlan
nosy: + ncoghlan
2008-07-25 15:26:30Antoine d'Otreppecreate