classification
Title: Add decorator_factory function to functools module
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.10
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: BTaskaya, ncoghlan, rhettinger, serhiy.storchaka, uriyyo
Priority: normal Keywords: patch

Created on 2020-11-24 20:34 by uriyyo, last changed 2021-01-05 11:34 by ncoghlan. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 23501 closed uriyyo, 2020-11-24 20:37
Messages (12)
msg381773 - (view) Author: Yurii Karabas (uriyyo) * (Python triager) Date: 2020-11-24 20:34
In the current python codebases, there are a lot of decorators with parameters, and their code in most cases contains a lot of code boilerplates. The purpose of this issue is to add `decorator_with_params` function to `functools` module. This function will allow using a wrapped function as a simple decorator and decorator with parameter. 

I believe the real example will bring more context, for instance, `typing._tp_cache` function can be rewritten from:
```
def _tp_cache(func=None, /, *, typed=False):
    """Internal wrapper caching __getitem__ of generic types with a fallback to
    original function for non-hashable arguments.
    """
    def decorator(func):
        cached = functools.lru_cache(typed=typed)(func)
        _cleanups.append(cached.cache_clear)

        @functools.wraps(func)
        def inner(*args, **kwds):
            try:
                return cached(*args, **kwds)
            except TypeError:
                pass
            return func(*args, **kwds)
        return inner

    if func is not None:
        return decorator(func)

    return decorator
```
To
```
@decorator_with_params
def _tp_cache(func=None, /, *, typed=False):
    """Internal wrapper caching __getitem__ of generic types with a fallback to
    original function for non-hashable arguments.
    """
    cached = functools.lru_cache(typed=typed)(func)
    _cleanups.append(cached.cache_clear)

    @functools.wraps(func)
    def inner(*args, **kwds):
        try:
            return cached(*args, **kwds)
        except TypeError:
            pass  # All real errors (not unhashable args) are raised below.
        return func(*args, **kwds)

    return inner
```

And `_tp_cache` can be used as:
```
    @_tp_cache(typed=True)
    def __getitem__(self, parameters):
        return self._getitem(self, parameters)
    ...
    @_tp_cache
    def __getitem__(self, parameters):
        return self._getitem(self, parameters)
```

Proposed implementation is quite simple (I used it in a lot of projects):
```
def decorator_with_params(deco):
    @wraps(deco)
    def decorator(func=None, /, *args, **kwargs):
        if func is not None:
            return deco(func, *args, **kwargs)

        def wrapper(f):
            return deco(f, *args, **kwargs)

        return wrapper

    return decorator
```

I believe it will be a great enhancement for the Python standard library and will save such important indentations.
msg381793 - (view) Author: Yurii Karabas (uriyyo) * (Python triager) Date: 2020-11-25 07:00
Add Raymond Hettinger because he is at `Expert Index` for `functools` module
msg382065 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-11-29 14:30
I would call it decorator_factory.
msg382068 - (view) Author: Yurii Karabas (uriyyo) * (Python triager) Date: 2020-11-29 14:59
I agree, decorator factory definitely a better name
msg382101 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2020-11-30 01:13
I've add Nick so that he can opine.  For me, I'm -0 on this one. It does factor-out a common pattern, but I find it less obvious what it does and would not enjoy debugging code that used this decorator.

The idea is definitely interesting and worth discussing.  If Nick doesn't come along shortly, please bring this up on python-ideas.
msg382120 - (view) Author: Yurii Karabas (uriyyo) * (Python triager) Date: 2020-11-30 08:31
Let's wait for Nick.

I have found great examples to show how decorator_factory can simplify things.

dataclasses.dataclass
```
def dataclass(cls=None, /, *, init=True, repr=True, eq=True, order=False,
              unsafe_hash=False, frozen=False):
    def wrap(cls):
        return _process_class(cls, init, repr, eq, order, unsafe_hash, frozen)

    # See if we're being called as @dataclass or @dataclass().
    if cls is None:
        # We're called with parens.
        return wrap

    # We're called as @dataclass without parens.
    return wrap(cls)
```
```
@functools.decorator_factory
def dataclass(cls, /, *, init=True, repr=True, eq=True, order=False,
              unsafe_hash=False, frozen=False):
    return _process_class(cls, init, repr, eq, order, unsafe_hash, frozen)
```

functools.lru_cache
```
def lru_cache(maxsize=128, typed=False):
    if isinstance(maxsize, int):
        # Negative maxsize is treated as 0
        if maxsize < 0:
            maxsize = 0
    elif callable(maxsize) and isinstance(typed, bool):
        # The user_function was passed in directly via the maxsize argument
        user_function, maxsize = maxsize, 128
        wrapper = _lru_cache_wrapper(user_function, maxsize, typed, _CacheInfo)
        wrapper.cache_parameters = lambda : {'maxsize': maxsize, 'typed': typed}
        return update_wrapper(wrapper, user_function)
    elif maxsize is not None:
        raise TypeError(
            'Expected first argument to be an integer, a callable, or None')

    def decorating_function(user_function):
        wrapper = _lru_cache_wrapper(user_function, maxsize, typed, _CacheInfo)
        wrapper.cache_parameters = lambda : {'maxsize': maxsize, 'typed': typed}
        return update_wrapper(wrapper, user_function)

    return decorating_function
```
```
@decorator_factory
def lru_cache(user_function, /, maxsize=128, typed=False):
    if isinstance(maxsize, int):
        # Negative maxsize is treated as 0
        if maxsize < 0:
            maxsize = 0
    elif maxsize is not None:
        raise TypeError(
            'Expected first argument to be an integer, a callable, or None')

    wrapper = _lru_cache_wrapper(user_function, maxsize, typed, _CacheInfo)
    wrapper.cache_parameters = lambda : {'maxsize': maxsize, 'typed': typed}
    return update_wrapper(wrapper, user_function)
```
msg382124 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-11-30 08:40
Would be nice to include these examples (and more if they exist) in the PR. It will help to see the benefit from the new feature and to test that it does not break anything.

I have no opinion yet, but multiple examples can convince me. Or not.
msg382188 - (view) Author: Yurii Karabas (uriyyo) * (Python triager) Date: 2020-11-30 20:05
I have posted this idea to python-ideas

Link to thread https://mail.python.org/archives/list/python-ideas@python.org/thread/OIRAB3QA4AAD3JGH2S5HMGCPDLGG7T52/
msg384263 - (view) Author: Yurii Karabas (uriyyo) * (Python triager) Date: 2021-01-03 11:48
What we should do with this issue?

There was no much discussion at python-ideas.

I have received only one -1 opinion.
msg384269 - (view) Author: Batuhan Taskaya (BTaskaya) * (Python committer) Date: 2021-01-03 13:09
I also agree with Raymond's opinion, and from the point that it didn't get much attention I'd assume not many people need it. So count me as -1
msg384382 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2021-01-05 11:23
I think the idea is a plausible and well-presented suggestion, but I'm afraid I'm still going to agree with the view that we shouldn't add this.

From a maintainability point of view, *generically* detecting the difference between "applied without parentheses as a decorator" and "called with parameters to produce a partial function to be used as a decorator" isn't quite as simple as just calling "callable(args[0])", since it's entirely possible for decorator factories to accept callables as parameters. We could try to document our way around that problem, but we wouldn't be able to eliminate it entirely.

Given the relative rarity of the use case, and the potential for subtle issues when the assumptions of the decorator_factory decorator aren't met, I'm happy to leave this task to inline mostly-but-not-completely-boilerplate code rather than trying to abstract it away.
msg384385 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2021-01-05 11:34
Sorry, I meant to include this comment as well:

The other filter I applied here was "Could someone figure out the boilerplate code themselves in less time than it would take them to find, read, and understand the documentation for the new helper function?"

I'm not sure the answer to that question is "Yes" when writing a decorator factory for the first time, but I am confident it is "Yes" when reading a decorator factory, and for anyone writing their second and subsequent factory (especially if they use the "def factory(decorated=None, /, *, keyword="only", params=True)" idiom the way dataclass and _tp_cache do).
History
Date User Action Args
2021-01-05 11:34:43ncoghlansetmessages: + msg384385
2021-01-05 11:24:00ncoghlansetstatus: open -> closed
resolution: rejected
messages: + msg384382

stage: patch review -> resolved
2021-01-03 13:09:40BTaskayasetnosy: + BTaskaya
messages: + msg384269
2021-01-03 11:48:13uriyyosetmessages: + msg384263
2020-11-30 20:05:25uriyyosetmessages: + msg382188
2020-11-30 08:40:56serhiy.storchakasetmessages: + msg382124
2020-11-30 08:31:05uriyyosetmessages: + msg382120
2020-11-30 01:13:24rhettingersetnosy: + ncoghlan
messages: + msg382101
2020-11-29 15:12:44uriyyosettitle: Add decorator_with_params function to functools module -> Add decorator_factory function to functools module
2020-11-29 14:59:50uriyyosetmessages: + msg382068
2020-11-29 14:30:25serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg382065
2020-11-25 07:00:07uriyyosetnosy: + rhettinger
messages: + msg381793
2020-11-24 20:37:55uriyyosetkeywords: + patch
stage: patch review
pull_requests: + pull_request22390
2020-11-24 20:34:37uriyyocreate