Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore missing attributes in functools.update_wrapper #47695

Closed
AntoinedOtreppe mannequin opened this issue Jul 25, 2008 · 20 comments
Closed

Ignore missing attributes in functools.update_wrapper #47695

AntoinedOtreppe mannequin opened this issue Jul 25, 2008 · 20 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@AntoinedOtreppe
Copy link
Mannequin

AntoinedOtreppe mannequin commented Jul 25, 2008

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
    Copy link
    Mannequin Author

    AntoinedOtreppe mannequin commented Jul 25, 2008

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

    @AntoinedOtreppe AntoinedOtreppe mannequin added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir labels Jul 25, 2008
    @benjaminp
    Copy link
    Contributor

    I think what you want is

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

    @ncoghlan
    Copy link
    Contributor

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

    @ncoghlan ncoghlan removed their assignment Jul 27, 2008
    @ncoghlan ncoghlan added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels Jul 27, 2008
    @ncoghlan ncoghlan changed the title functools.update_wrapper bug Ignore missing attributes in functools.update_wrapper Jul 27, 2008
    @AntoinedOtreppe
    Copy link
    Mannequin Author

    AntoinedOtreppe mannequin commented Jul 28, 2008

    Thank you for considering this report :)

    @pitrou
    Copy link
    Member

    pitrou commented Jul 28, 2008

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

    @findepi
    Copy link
    Mannequin

    findepi mannequin commented Aug 21, 2008

    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
    Copy link
    Contributor

    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
    Copy link
    Mannequin

    eklitzke mannequin commented Jan 14, 2010

    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
    Copy link
    Member

    pitrou commented Jan 15, 2010

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

    @eklitzke
    Copy link
    Mannequin

    eklitzke mannequin commented Jan 16, 2010

    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
    Copy link
    Member

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

    @pitrou
    Copy link
    Member

    pitrou commented Jan 16, 2010

    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
    Copy link
    Mannequin

    freakboy3742 mannequin commented Mar 12, 2010

    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
    Copy link
    Mannequin

    tifv mannequin commented May 2, 2010

    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
    Copy link
    Mannequin

    tifv mannequin commented May 3, 2010

    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
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Jul 18, 2010

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

    @ncoghlan
    Copy link
    Contributor

    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 ncoghlan self-assigned this Jul 18, 2010
    @ncoghlan
    Copy link
    Contributor

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

    @YanivAknin
    Copy link
    Mannequin

    YanivAknin mannequin commented Feb 25, 2012

    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
    Copy link
    Member

    merwok commented Feb 26, 2012

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants