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

decorator syntax: allow testlist instead of just dotted_name #63859

Closed
james mannequin opened this issue Nov 20, 2013 · 20 comments
Closed

decorator syntax: allow testlist instead of just dotted_name #63859

james mannequin opened this issue Nov 20, 2013 · 20 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement

Comments

@james
Copy link
Mannequin

james mannequin commented Nov 20, 2013

BPO 19660
Nosy @gvanrossum, @ncoghlan, @ericvsmith, @benjaminp, @merwok, @ericsnowcurrently, @berkerpeksag, @dirn, @jdemeyer, @gvanrossum, @Vgr255, @brandtbucher
Superseder
  • bpo-39702: PEP 614: Relaxing Grammar Restrictions On Decorators
  • Files
  • decorator-syntax.patch
  • decorator-syntax.patch
  • decorator-syntax.patch
  • 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 = None
    closed_at = <Date 2020-02-20.16:20:55.152>
    created_at = <Date 2013-11-20.01:27:05.121>
    labels = ['interpreter-core', 'type-feature']
    title = 'decorator syntax: allow testlist instead of just dotted_name'
    updated_at = <Date 2020-02-20.16:20:55.151>
    user = 'https://bugs.python.org/james'

    bugs.python.org fields:

    activity = <Date 2020-02-20.16:20:55.151>
    actor = 'brandtbucher'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-02-20.16:20:55.152>
    closer = 'brandtbucher'
    components = ['Interpreter Core']
    creation = <Date 2013-11-20.01:27:05.121>
    creator = 'james'
    dependencies = []
    files = ['32717', '32741', '32745']
    hgrepos = []
    issue_num = 19660
    keywords = ['patch']
    message_count = 20.0
    messages = ['203456', '203531', '203532', '203538', '203539', '203554', '203555', '203569', '203728', '203743', '239697', '271744', '271750', '271755', '271758', '271762', '271764', '271767', '321307', '334141']
    nosy_count = 15.0
    nosy_names = ['gvanrossum', 'ncoghlan', 'eric.smith', 'benjamin.peterson', 'eric.araujo', 'infinity0', 'eric.snow', 'berker.peksag', 'dirn', 'jdemeyer', 'Guido.van.Rossum', 'james', 'peyton', 'abarry', 'brandtbucher']
    pr_nums = []
    priority = 'normal'
    resolution = 'duplicate'
    stage = 'resolved'
    status = 'closed'
    superseder = '39702'
    type = 'enhancement'
    url = 'https://bugs.python.org/issue19660'
    versions = ['Python 3.6']

    @james
    Copy link
    Mannequin Author

    james mannequin commented Nov 20, 2013

    Decorator syntax currently allows only a dotted_name after the @. As far as I can tell, this was a gut-feeling decision made by Guido. [1]

    I spoke with Nick Coghlan at PyTexas about this, and he suggested that if someone did the work, there might be interest in revisiting this restriction.

    The attached patch allows any testlist to follow the @.

    The following are now valid:

    @(lambda x:x)
    def f():
    pass

    @(spam if p else eggs)
    def f():
        pass
    
    @spam().ham().eggs()
    def f():
        pass

    [1] http://mail.python.org/pipermail/python-dev/2004-August/046711.html

    @james james mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement labels Nov 20, 2013
    @merwok
    Copy link
    Member

    merwok commented Nov 20, 2013

    Thanks for this! Tests should exercise the now-valid syntaxes, which also need documentation.

    @merwok
    Copy link
    Member

    merwok commented Nov 20, 2013

    On second thought, as this patch allows one form that Guido doesn’t want (bar().foo()), maybe there should be a discussion on python-ideas.

    @ncoghlan
    Copy link
    Contributor

    Nice! As a syntax change (albeit a minor one), I believe this will require a PEP for Python 3.5.

    I know Guido indicated he was OK with relaxing the current restrictions, but I don't remember exactly where he said it, or how far he was willing to relax them.

    @gvanrossum
    Copy link
    Member

    I don't feel very strongly, but I do think that most of the things the new syntax allows are not improvements -- they make the decorator harder to read. It was intentional to force you to compute a variable before you can use it as a decorator, e.g.

    spamify = (spam if p else eggs)
    
    @spamify
    def f():
        pass

    @ericsnowcurrently
    Copy link
    Member

    they make the decorator harder to read.

    I agree.

    @james
    Copy link
    Mannequin Author

    james mannequin commented Nov 21, 2013

    I see this as removing a restriction and a special-case from the
    decorator syntax (noting, of course, that these were introduced
    deliberately.)

    In terms of whether the new forms are improvements, my preference is to
    leave this up to the judgement of the programmer, moderated of course by
    their prevailing coding guide.

    I would argue that this change does not force any additional complexity
    on the programmer (who is free to take or leave it) or on the
    interpreter (- the straightforwardness of the patch corroborates this.)

    I would also argue that there are certainly cases where, in the midst of
    some large codebase, the dotted_name restriction may seem a bit arbitrary.

    This is likely true for:

    class Foo:
        def bar(self, func):
            return func
    
        @staticmethod
        def baz(func):
            return func
    	
        @staticmethod
        def quux():
            def dec(func):
                return func
            return dec
    
    # invalid
    @Foo().bar
    def f(): pass
    
    # valid
    @Foo.baz
    def f(): pass
    
    # valid
    @Foo.quux()
    def f(): pass

    For completeness' sake, I have attached a patch with an additional unit
    test and amended documentation.

    Should we proceed with writing a PEP for Python 3.5?

    @ncoghlan
    Copy link
    Contributor

    Yes, a PEP for 3.5 on this will be valuable, whether it's accepted or not
    (although I personally favour moving these restrictions out of the compiler
    and into the PEP-8 style guide).

    If I recall the past python-ideas threads correctly, the main objections to
    the current syntax restrictions were:

    • you can't look up decorators through a registry by default, since
      "@registry[name]" is disallowed
    • it's not really a limitation anyway, since a pass through function still
      lets you write whatever you want:
        def deco(x): return x
    @deco(registry[name])
    def f(): ...
    

    Now that the precedent of keeping decorator expressions simple has been
    well and truly established, simplification of the grammar is the main
    reason removing the special casing of decorator syntax from the compilation
    toolchain appeals to me.

    @benjaminp
    Copy link
    Contributor

    I think the complexity delta in the grammar is exactly 0.

    @ericvsmith
    Copy link
    Member

    While I think that the dotted_name restriction should be relaxed and it should instead be a style guide issue, I have to agree with Benjamin here: the difference in grammar complexity is zero and shouldn't drive the decision.

    @infinity0
    Copy link
    Mannequin

    infinity0 mannequin commented Mar 31, 2015

    Yes, please get rid of this restriction. It's trivial to get around - you don't even need to define your own "pass-through", one already exists in the standard library:

    >>> @(lambda: [lambda x: x][0])()
      File "<stdin>", line 1
        @(lambda: [lambda x: x][0])()
         ^
    SyntaxError: invalid syntax
    
    >>> from functools import partial as _
    >>> @_( (lambda: [lambda x: x][0])() )
    ... def f(x): return x*x
    ... 
    >>> f(3)
    9

    I don't know the rational behind disallowing bar().foo(), but it is the use-case I have in mind - something like:

    @MainDecoratorFactory(params).tweakedDecorator(tweak_params)
    def f(x):
      pass

    or even

    @MainDecoratorFactory(params).\
    tweakedDecorator(tweak_params)
    def f(x):
    pass

    It should be no more controversial than chaining decorators.

    The alternative with the current restrictions would be tweakedDecorator(MainDecorator(params), tweak_params) which is more ugly and visually separates the "tweak" concepts.

    It's not appropriate to merge MainDecoratorFactory and the tweaks together: there are several MainDecoratorFactories taking care of one main concern; they don't care about the tweaks. And vice versa; the tweaks shouldn't care about the main decorators.

    @gvanrossum
    Copy link
    Member

    Nobody has posted a real use case. All the examples are toys. What are the
    real use cases that are blocked by this? Readability counts!

    @Vgr255
    Copy link
    Mannequin

    Vgr255 mannequin commented Jul 31, 2016

    TL;DR - Use case is dynamic decorators. Not all of the syntax would make sense, see below.

    The main benefit of this feature would be for dynamic decorators (as was evidenced from others in this issue). In a project I contribute to, we use dynamic decorators to set a function as being a command, and we use the object (a wrapper around the function) directly, so we need a bit more boilerplate around the place.

    Ultimately, we would definitely use such a feature (just the '@spam().eggs()' part; we'd have no use for the other ones) , but we probably won't notice its absence either; we've worked around it for years, after all.

    As far as readability goes, I think allowing only the '@spam().eggs()' version would actually improve readability quite a bit, by reducing the need to separate the decorator assignment in two (or more) parts. I can see the desire to have a '@spam[eggs]' kind of syntax though, again for the dynamic decorators case.

    I see no reason to allow creating lambdas or conditional expressions inside decorators expressions. If anything, that'll encourage anti-patterns, whereas e.g. '@spam(value=eggs)' would be more readable, and would let the decorator decide what it wants to do with the value.

    And if your decorator can be expressed as a lambda, why don't you put that in/below the function? Surely it's less work than writing the lambda everytime ;)

    @gvanrossum
    Copy link
    Member

    Could you link to an example decorator definition and its call site(s)
    that would benefit? I'm lacking the imagination to understand what a
    "dynamic decorator" might be. @spam().eggs() is not enough to help me
    understand -- I understand quite well what syntax people are
    requesting, but I am unclear on what they actually want to do with it.
    I worry there's some extreme use of higher-order functions here that
    would just get in the way of readability, but a real-world example
    might dispell my fear. (That's what I meant when I said "use case".)

    @Vgr255
    Copy link
    Mannequin

    Vgr255 mannequin commented Aug 1, 2016

    Sure, here goes; this is an IRC game bot which I contribute to. Apologies for the long links, it's the only way to make sure this consistently points to the same place regardless of future commits.

    The 'cmd' decorator we use is defined at https://github.com/lykoss/lykos/blob/1852bf2c442d707ba0cbc16e8c9e012bcbc4fcc5/src/decorators.py#L67 - we use its __call__ method to add the function to it; see next link.

    How it's used: https://github.com/lykoss/lykos/blob/1852bf2c442d707ba0cbc16e8c9e012bcbc4fcc5/src/wolfgame.py#L9113 - ideally, a syntax such as the following would be nice for these definitions:

    ­@cmd("myrole", <keyword arguments here>).set
    def myrole(cli, nick, chan, rest):
    # ... do stuff here ...

    Historically (we used an arcane closure-based version that no one understood), we could call that function after directly, like any normal function. Now, though, we have to call it like this: https://github.com/lykoss/lykos/blob/1852bf2c442d707ba0cbc16e8c9e012bcbc4fcc5/src/wolfgame.py#L764

    I'd like to state again that, while we'd use this new syntax, we've already worked around this lack of syntax. Whatever comes out of this, we won't be negatively affected, but decorators are meant to bring whatever alters the function right where it starts, so having syntax that eases that would make sense (to me, anyway).

    @gvanrossum
    Copy link
    Member

    OK, so if you wanted to be able to call myrole(...) instead of
    myrole.caller, why doesn't cmd.__call__ return self.caller rather than
    just self?

    @Vgr255
    Copy link
    Mannequin

    Vgr255 mannequin commented Aug 1, 2016

    We want to be able to access the instance attributes (as is done e.g. here: https://github.com/lykoss/lykos/blob/1852bf2c442d707ba0cbc16e8c9e012bcbc4fcc5/src/wolfgame.py#L9761 ). I realize we can set the attributes directly on the functions, but we've decided to not do that (it's a style thing, really). Although I guess a class method which then returns our desired method could work out for us.

    While I still think that this kind of syntax might be useful for dynamic decorators (I know I'd use that when playing with decorators), I'm afraid I'm out of concrete examples to send your way.

    @gvanrossum
    Copy link
    Member

    OK, maybe someone else wants to provide a real-world example.
    Otherwise I am really going to close this (again).

    @jdemeyer
    Copy link
    Contributor

    jdemeyer commented Jul 9, 2018

    Real world example where this actually came up:

    jupyter-widgets/ipywidgets#430 (comment)

    @jdemeyer
    Copy link
    Contributor

    There is again some discussion about this at https://discuss.python.org/t/why-are-some-expressions-syntax-errors/420

    @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
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    8 participants