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

Let lru_cache be used as a decorator with no arguments #80953

Closed
rhettinger opened this issue May 2, 2019 · 10 comments
Closed

Let lru_cache be used as a decorator with no arguments #80953

rhettinger opened this issue May 2, 2019 · 10 comments
Labels
3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@rhettinger
Copy link
Contributor

BPO 36772
Nosy @rhettinger, @scoder, @ericvsmith, @giampaolo, @DinoV, @njsmith, @ericsnowcurrently, @serhiy-storchaka, @pablogsal
PRs
  • bpo-36772 Allow lru_cache to be used as decorator without making a function call #13048
  • 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 2019-05-26.19:14:29.100>
    created_at = <Date 2019-05-02.00:40:09.114>
    labels = ['3.8', 'type-bug', 'library']
    title = 'Let lru_cache be used as a decorator with no arguments'
    updated_at = <Date 2019-05-26.19:14:29.099>
    user = 'https://github.com/rhettinger'

    bugs.python.org fields:

    activity = <Date 2019-05-26.19:14:29.099>
    actor = 'rhettinger'
    assignee = 'none'
    closed = True
    closed_date = <Date 2019-05-26.19:14:29.100>
    closer = 'rhettinger'
    components = ['Library (Lib)']
    creation = <Date 2019-05-02.00:40:09.114>
    creator = 'rhettinger'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 36772
    keywords = ['patch']
    message_count = 10.0
    messages = ['341239', '341246', '341255', '341266', '341319', '341391', '341392', '341446', '341569', '343575']
    nosy_count = 9.0
    nosy_names = ['rhettinger', 'scoder', 'eric.smith', 'giampaolo.rodola', 'dino.viehland', 'njs', 'eric.snow', 'serhiy.storchaka', 'pablogsal']
    pr_nums = ['13048']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue36772'
    versions = ['Python 3.8']

    @rhettinger
    Copy link
    Contributor Author

    Follow the lead of the dataclasses module and allow lru_cache() to be used as a straight decorator rather than as a function that returns a decorator. Both of these would now be supported:

        @lru_cache
        def f(x):
            ...
    
        @lru_cache(maxsize=256)
        def f(x):
            ...

    @rhettinger rhettinger added 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels May 2, 2019
    @serhiy-storchaka
    Copy link
    Member

    I do not like mixing decorators and decorator fabrics. But this can of worms has already been open by dataclasses. The question is whether we want to extend this design to the rest of the stdlib. lru_cache() resisted this change for a long time.

    @ericvsmith
    Copy link
    Member

    The 90% use case with dataclasses is satisfied with just "@DataClass", and perhaps the target user there is less sophisticated.

    But the main difference between @DataClass and @lru_cache is that all of the parameters to @DataClass are keyword-only. Which I did because they are mostly all booleans, and "@DataClass(True, False, True)" isn't very helpful.

    That's not the case with @lru_cache. Whether that makes a difference for this issue in particular, I'm not sure.

    I agree that this violates "only one way to do it". But I haven't seen anyone use "@DataClass()", and I've also never seen anyone be confused by the fact that once you want parameters, you change to using parens. For the dataclass case, I think that working both ways has been helpful.

    @scoder
    Copy link
    Contributor

    scoder commented May 2, 2019

    I'm generally ok with such APIs. It seems needless to require

      @lru_cache()
      def f(): ...

    when a simple decorator would suffice. I think I might decide otherwise in cases where almost all usages require arguments, but if the no-arguments case is common (and there is no ambiguity in the arguments), then I prefer not requiring parentheses.

    @rhettinger
    Copy link
    Contributor Author

    At Pycon today, I mostly got all positive feedback on this. Apparently py.test already follows this pattern. Another developer said they found the () to be distracting and would welcome not having to use them.

    @ericsnowcurrently
    Copy link
    Member

    FWIW, I've followed this pattern (one function is both decorator and factory) in my own code for quite a while. I've never found it confusing nor has anyone else (that I'm aware) that has used those decorators.

    One reason I've done decorators this way is because the empty parentheses are a visual distraction to readers. They also imply to readers that more is going on than really is. So I'm in favor of Raymond's plan.

    @ericsnowcurrently
    Copy link
    Member

    As to the issue of positional vs. keyword arguments, keyword arguments make the implementation easier in some cases. Otherwise I haven't seen positional arguments cause much of a problem.

    @serhiy-storchaka
    Copy link
    Member

    To clarify, I am not opposing this feature. I had doubts because if the memory does not betray me similar propositions for lru_cache or other decorator fabrics were rejected in past. But times change.

    @pablogsal
    Copy link
    Member

    I am +1 to this. Making it easier for the case of decorator factories that are called with all the defaults is a very common pattern in the wild. There are even known idioms for how to reduce the indentation of the naive approach (returning partials of the factory). In particular for lru_cache, my experience is that every time I teach this feature, people are confused initially about the '()' at the end of the call.

    @rhettinger
    Copy link
    Contributor Author

    New changeset b821868 by Raymond Hettinger in branch 'master':
    bpo-36772 Allow lru_cache to be used as decorator without making a function call (GH-13048)
    b821868

    @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
    3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants