4
\$\begingroup\$

I know you must've been already tired of Python decorators, but this one seems new! There is a common practice of making decorators with arguments. But Flat is better than nested, so let's flatten it up:

def decorator_with_args(arg):

    def wrapper(*args, **kwargs):
        print('decorated with arg: {}'.format(arg))
        return wrapper.func(*args, **kwargs)

    def wrapper_creator(decorable):
        wrapper.func = decorable
        return functools.wraps(decorable)(wrapper)

    return wrapper_creator

Wrapped func is passed as an attribute, arg - via closure. They don't get overwritten with multiple consequent calls to decorator_with_args.

But not chained ones - functools.wraps will update wrapper's __dict__, overwriting its func attribute like this:

@decorator_with_args('This decorator will be applied')
@decorator_with_args('Others will not')
def gotcha():
    pass

So, attributes are not a good place to store our func. But closure seems to be. Let's use it:

def decorator_with_args(arg):
    func = None

    def wrapper(*args, **kwargs):
        print('decorated with arg: {}'.format(arg))
        return func(*args, **kwargs)

    def wrapper_creator(decorable):
        nonlocal func
        func = decorable
        return functools.wraps(decorable)(wrapper)

    return wrapper_creator

Now it looks good to me! Does it to you? Is it pythonic, idiomatic, correct and nicely performing?

And a real example of making something useful (this is a part to be reviewed):

import functools
import logging
import logging.config


def temp_loglevel(level):
    """
    A decorator for setting temporary loglevel for the scope of
    the wrapped function, then reverting it back on wrapper exit

    Arguments:
        level: new logging level to set
    """
    logger = logging.getLogger()
    func = None

    def wrapper(*args, **kwargs):
        old_level = logger.level
        logger.setLevel(level)
        rv = func(*args, **kwargs)
        logger.setLevel(old_level)
        return rv

    def wrapper_maker(decorable):
        nonlocal func
        func = decorable
        logger.warn(
            "temp_loglevel used, don't forget to "
            "remove it when debugging is done"
        )

        return functools.wraps(decorable)(wrapper)

    return wrapper_maker


logging.config.dictConfig(dict(
    version = 1,
    disable_existing_loggers = False,
    formatters = {
        'default': {
            'format': '%(asctime)-15s [%(levelname)s] %(message)s'
        }
    },
    handlers = {
        'std': {
            'level': 'DEBUG',
            'class': 'logging.StreamHandler',
            'formatter': 'default'
        }
    },
    root = {
        'handlers': ['std'],
        'propagate': False,
        'level': 'INFO'
    }
))


@temp_loglevel(logging.DEBUG)
def print_debug():
    logging.getLogger().debug('This should be visible')


if __name__ == '__main__':
    print_debug()

I would be glad if you put some critique of my code styling as well!

\$\endgroup\$
4
  • 2
    \$\begingroup\$ Welcome to Code Review! I don't understand what code we're reviewing, though. This decorator and it's argument doesn't seem to accomplish anything. It seems that you are asking for opinions about the general practice of splitting up arguments into multiple decorators, in which case I am voting to close this question as hypothetical. \$\endgroup\$ Jan 18, 2019 at 8:32
  • \$\begingroup\$ @200_success Oh, sorry. It is a bit stripped off example to focus the attention on one aspect of the implementation - the decorator itself (not the wrapper), and to facilitate things for those willing to run it (to test their own ideas or use cases). I could add my real production code if needed. Should I? \$\endgroup\$
    – ogurets
    Jan 18, 2019 at 9:34
  • 3
    \$\begingroup\$ @ogurets Yes, because unlike Stack Overflow, Code Review needs to look at concrete code in a real context. Please see Why is hypothetical example code off-topic for CR?. \$\endgroup\$ Jan 18, 2019 at 9:40
  • \$\begingroup\$ @MathiasEttinger Done! \$\endgroup\$
    – ogurets
    Jan 18, 2019 at 10:06

1 Answer 1

1
\$\begingroup\$

I've found a couple bugs - lack of exception support and it doesn't work with generators. Well, it does in a way, but not the way someone would expect it to.

To fix aforementioned issues, decorator have been changed into this (nevermind Python 2.7 compatibility modifications):

def temp_loglevel(level):
    """
    Decorator to set temporary loglevel for the scope of this function
    (then revert it back on wrapper exit)

    Arguments:
        level: new logging level to set
    """
    logger = logging.getLogger()
    p_closure = dict(func=None)

    def wrapper(*args, **kwargs):
        old_level = logger.level
        logger.setLevel(level)
        try:
            rv = p_closure['func'](*args, **kwargs)
            if isinstance(rv, types.GeneratorType):
                def wrapper_gen():
                    # wrapper's finally block overrides initial setLevel after
                    # returning a generator object
                    logger.setLevel(level)
                    try:
                        for x in rv:
                            yield x
                    finally:
                        logger.setLevel(old_level)

                return wrapper_gen()
            else:
                return rv
        finally:
            logger.setLevel(old_level)

    def wrapper_maker(decorable):
        p_closure['func'] = decorable
        logger.warn(
            "temp_loglevel used, don't forget to "
            "remove it when debugging is done"
        )

        return functools.wraps(decorable)(wrapper)

    return wrapper_maker

P.S. After thinking about how ugly would a flat version of wrapper_gen be, I've made a mental note to myself: "flat is better than nested", but if you absolutely need to make something nested, do it FFS!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.