Ignore missing attributes in functools.update_wrapper

[AntoinedOtreppe]

BPO	3445
Nosy	@ncoghlan, @pitrou, @benjaminp, @merwok, @briancurtin, @tifv, @eklitzke, @freakboy3742
Files	fix.patch: trivial change to ignore missing "asssigned" attributes fix.patch: updated patch update_wrapper-ignore-missing-attributes.patch: patch with test updated (apply to trunk)

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ncoghlan'
closed_at = <Date 2010-08-17.06:20:13.801>
created_at = <Date 2008-07-25.15:26:30.282>
labels = ['type-feature', 'library']
title = 'Ignore missing attributes in functools.update_wrapper'
updated_at = <Date 2021-10-23.21:13:15.655>
user = 'https://bugs.python.org/AntoinedOtreppe'

bugs.python.org fields:

activity = <Date 2021-10-23.21:13:15.655>
actor = 'yaubi'
assignee = 'ncoghlan'
closed = True
closed_date = <Date 2010-08-17.06:20:13.801>
closer = 'ncoghlan'
components = ['Library (Lib)']
creation = <Date 2008-07-25.15:26:30.282>
creator = "Antoine d'Otreppe"
dependencies = []
files = ['15865', '15903', '17184']
hgrepos = []
issue_num = 3445
keywords = ['patch', 'needs review']
message_count = 20.0
messages = ['70256', '70320', '70328', '70331', '70352', '71677', '71695', '97745', '97829', '97862', '97908', '97909', '100933', '104787', '104832', '110679', '110691', '114101', '154271', '154301']
nosy_count = 12.0
nosy_names = ['ncoghlan', 'pitrou', 'benjamin.peterson', 'eric.araujo', "Antoine d'Otreppe", 'findepi', 'brian.curtin', 'july', 'eklitzke', 'freakboy3742', 'Yaniv.Aknin', 'BreamoreBoy']
pr_nums = []
priority = 'normal'
resolution = 'accepted'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue3445'
versions = ['Python 3.2']

[AntoinedOtreppe]

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__'

[benjaminp]

I think what you want is

functools.update_wrapper(myWrapper, str.split, ('__name__', '__doc__'))

[ncoghlan]

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).

[AntoinedOtreppe]

Thank you for considering this report :)

[pitrou]

Another possibility would be for methods to also get the __name__ and
__module__ attributes. I don't see any counter-indication to it.

[findepi]

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.

[ncoghlan]

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.

[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

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.

[pitrou]

The patch should come with an unit test (in Lib/test/test_functools.py).

[eklitzke]

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.

[briancurtin]

In your test, the more common convention is to use assertFalse(foo) instead of assert_(not foo).

[pitrou]

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.

[freakboy3742]

As an extra data point: we just hit this problem in Django ticket bpo-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.

[tifv]

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.)

[tifv]

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,)

[BreamoreBoy]

Is there anyone who can provide this issue with a bit of TLC as it's almost the 2nd birthday?

[ncoghlan]

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.

[ncoghlan]

Implemented in r84132 (not based on this patch though).

[YanivAknin]

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

[merwok]

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.