Re: Decorators not worth the effort

Discussion in 'Python' started by Jean-Michel Pichavant, Sep 14, 2012.

  1. ----- Original Message -----
    > On Sep 14, 3:54 am, Jean-Michel Pichavant <>
    > wrote:
    > > I don't like decorators, I think they're not worth the mental
    > > effort.

    >
    > Because passing a function to a function is a huge cognitive burden?
    > --
    > http://mail.python.org/mailman/listinfo/python-list
    >


    I was expecting that. Decorators are very popular so I kinda already know that the fault is mine. Now to the reason why I have troubles writing them, I don't know. Every time I did use decorators, I spent way too much time writing it (and debugging it).

    I wrote the following one, used to decorate any function that access an equipment, it raises an exception when the timeout expires. The timeout is adapted to the platform, ASIC of FPGA so people don't need to specify everytime one timeout per platform.

    In the end it would replace

    def boot(self, timeout=15):
    if FPGA:
    self.sendCmd("bootMe", timeout=timeout*3)
    else:
    self.sendCmd("bootMe", timeout=timeout)

    with

    @timeout(15)
    def boot(self, timeout=None):
    self.sendCmd("bootMe", timeout)

    I wrote a nice documentation with sphinx to explain this, how to use it, how it can improve code. After spending hours on the decorator + doc, feedback from my colleagues : What the F... !!

    Decorators are very python specific (probably exists in any dynamic language though, I don't know), in some environment where people need to switch from C to python everyday, decorators add python magic that not everyone is familiar with. For example everyone in the team is able to understand and debug the undecorated version of the above boot method. I'm the only one capable of reading the decorated version. And don't flame my colleagues, they're amazing people (just in case they're reading this :p) who are not python developers, more of users.

    Hence my original "decorators are not worth the mental effort". Context specific I must admit.

    Cheers,

    JM

    PS : Here's the decorator, just to give you an idea about how it looks. Small piece of code, but took me more than 2 hours to write it. I removed somesensible parts so I don't expect it to run.

    class timeout(object):
    """Substitute the timeout keyword argument with the appropriate value"""
    FACTORS = {
    IcebergConfig().platform.ASIC : 1,
    IcebergConfig().platform.FPGA : 3,
    }

    def __init__(self, asic, fpga=None, palladium=None):
    self.default = asic
    self.fpga = fpga

    def _getTimeout(self):
    platform = config().platform
    factor = self.FACTORS[platform.value]
    timeout = {
    platform.ASIC : self.default*factor,
    platform.FPGA : self.fpga or self.default*factor,
    }[platform.value]
    return timeout

    def __call__(self, func):
    def decorated(*args, **kwargs):
    names, _, _, defaults = inspect.getargspec(func)
    defaults = defaults or []
    if 'timeout' not in names:
    raise ValueError('A "timeout" keyword argument is required')
    if 'timeout' not in kwargs: # means the timeout keyword arg is notin the call
    index = names.index('timeout')
    argsLength = (len(names) - len(defaults))
    if index < argsLength:
    raise NotImplementedError('This decorator does not support non keyword "timeout" argument')
    if index > len(args)-1: # means the timeout has not be passed using a pos argument
    timeoutDef = defaults[index-argsLength]
    if timeoutDef is not None:
    _log.warning("Decorating a function with a default timeout value <> None")
    kwargs['timeout'] = self._getTimeout()
    else:
    _log.warning('Timeout value specified during the call, please check "%s" @timeout decorator.' % func.__name__)
    ret = func(*args, **kwargs)
    return ret
    return decorated
     
    Jean-Michel Pichavant, Sep 14, 2012
    #1
    1. Advertising

  2. Am 14.09.2012 11:28, schrieb Jean-Michel Pichavant:
    > Decorators are very popular so I kinda already know that the
    > fault is mine. Now to the reason why I have troubles writing
    > them, I don't know. Every time I did use decorators, I spent
    > way too much time writing it (and debugging it).
    >
    > I wrote the following one, used to decorate any function that access
    > an equipment, it raises an exception when the timeout expires. The
    > timeout is adapted to the platform, ASIC of FPGA so people don't need
    > to specify everytime one timeout per platform.
    >
    > In the end it would replace
    >
    > def boot(self, timeout=15):
    > if FPGA:
    > self.sendCmd("bootMe", timeout=timeout*3)
    > else:
    > self.sendCmd("bootMe", timeout=timeout)
    >
    > with
    >
    > @timeout(15)
    > def boot(self, timeout=None):
    > self.sendCmd("bootMe", timeout)
    >
    > I wrote a nice documentation with sphinx to explain this, how to use
    > it, how it can improve code. After spending hours on the decorator +
    > doc, feedback from my colleagues : What the F... !!


    Quite honestly: I think like your colleagues in this case and that in
    this case the decorator doesn't improve the code. Instead, I would
    probably have added a _get_timeout() function that takes care of
    adjusting the argument passed to the function according to the
    underlying hardware target.

    To be less abstract, the particular problem I have with your approach is
    that I can't even guess what your code means, let alone what parameters
    it actually takes. If you had written

    @default_timeout(15)
    def boot(self, timeout=None):

    instead, I would have been able to guess. OTOH, then again I would have
    wondered why you used a decorator to create a default argument when
    there is builtin support for specifying default arguments for functions.

    Maybe you could get away with a decorator like this:

    @adjust_timeout
    def boot(self, timeout=2.5):

    The idea is that the decorator modifies the timeout value passed to the
    function (or maybe just modifies the default value?) according to the
    underlying hardware.


    > Decorators are very python specific (probably exists in any dynamic
    > language though, I don't know), in some environment where people need
    > to switch from C to python everyday, decorators add python magic that
    > not everyone is familiar with.


    The same could be said for classes, iterators, significant whitespace,
    docstrings, lambdas. I think that this was just a bad example but it
    doesn't prove that decorators are worthless. Decorators are useful tools
    if they do something to a function, like doing something before or after
    the actual code, or modifying the context in which the code is called.
    Just setting a default parameter is possible as you have proved, but
    it's IMHO not a good use case.

    A bit more specific to your case, adding a timeout decorator would
    actually make much more sense if it transparently invoked the actual
    function in a second thread and the calling thread stops waiting for
    completion and raises an error after that timeout. This has the distinct
    advantage that the code doing the actual communication doesn't have any
    timeout handling code inside.

    I'm currently doing something similar here though I only monitor a TCP
    connection that is used for some telnet-style requests. Every function
    making a request over TCP is decorated with @_check_connection. That
    decorator does two things:
    1. It checks for an existing fatal connection error.
    2. It runs the request and filters resulting errors for fatal connection
    errors.

    The decorator looks like this:

    def _check_connection(fn):
    @functools.wraps(fn)
    def wrapper(self, *args, **kwargs):
    # check for sticky connection errors
    if self._connection_error:
    raise self._connection_error
    # run actual function
    try:
    return fn(self, *args, **kwargs)
    catch RequestFailed:
    # The other side signalled a failure, but
    # further requests can still succeed.
    raise
    catch ConnectionError, e:
    # The connection is broken beyond repair.
    # Store sticky connection and forward.
    self._connection_error = e
    raise
    return wrapper

    I have had other programmers here write such requests and they blindly
    copied the decorator from existing code. This works because the code
    inside that converts/formats/parses the inputs and outputs is completely
    unaware of the connection monitoring. Otherwise, I don't think anyone
    could explain what this decorator does, but they don't have to
    understand it either. It just works.

    I wish you a nice weekend!

    Uli
     
    Ulrich Eckhardt, Sep 14, 2012
    #2
    1. Advertising

  3. On Fri, 14 Sep 2012 11:28:22 +0200, Jean-Michel Pichavant wrote:

    > PS : Here's the decorator, just to give you an idea about how it looks.
    > Small piece of code, but took me more than 2 hours to write it. I
    > removed some sensible parts so I don't expect it to run.


    [snip timeout class]

    Holy over-engineering Batman!!!

    No wonder you don't think much of decorators, if this example of overkill
    is what you consider typical of them. It does much, much more than the
    simple code you were replacing:

    def boot(self, timeout=15):
    if FPGA:
    self.sendCmd("bootMe", timeout=timeout*3)
    else:
    self.sendCmd("bootMe", timeout=timeout)

    # becomes:

    @timeout(15)
    def boot(self, timeout=None):
    self.sendCmd("bootMe", timeout)


    Most of my decorator functions are under a dozen lines. And that's the
    complicated ones!

    Here's my solution to the example you gave:




    # Untested!
    def timeout(t=15):
    # Decorator factory. Return a decorator to actually do the work.
    if FPGA:
    t *= 3
    def decorator(func):
    @functools.wraps(func)
    def inner(self, timeout):
    self.sendCmd("bootMe", timeout=t)
    return inner
    return decorator


    I reckon that will pretty much do what your example showed. Of course,
    once you start adding more and more functionality above the simple code
    shown above (arbitrary platforms, argument checking of the decorated
    function, logging, etc.) you're going to get a much more complex
    decorator. On the other hand, YAGNI.

    http://en.wikipedia.org/wiki/You_ain't_gonna_need_it


    --
    Steven
     
    Steven D'Aprano, Sep 14, 2012
    #3
  4. Jean-Michel Pichavant

    Tim Chase Guest

    On 09/14/12 07:01, Steven D'Aprano wrote:
    > [snip timeout class]
    >
    > Holy over-engineering Batman!!!
    >
    > No wonder you don't think much of decorators,

    [snip]

    > Most of my decorator functions are under a dozen lines. And that's the
    > complicated ones!



    As are mine, and a sizable chunk of those under-a-dozen-lines are
    somewhat boilerplate like using @functools.wraps inside, actual def
    of the function, and returning that function. :)

    -tkc
     
    Tim Chase, Sep 14, 2012
    #4
  5. Jean-Michel Pichavant

    Steve Howell Guest

    On Sep 14, 6:05 am, Tim Chase <> wrote:
    > On 09/14/12 07:01, Steven D'Aprano wrote:> [snip timeout class]
    >
    > > Holy over-engineering Batman!!!

    >
    > > No wonder you don't think much of decorators,

    >
    > [snip]
    >
    > > Most of my decorator functions are under a dozen lines. And that's the
    > > complicated ones!

    >
    > As are mine, and a sizable chunk of those under-a-dozen-lines are
    > somewhat boilerplate like using @functools.wraps inside, actual def
    > of the function, and returning that function. :)
    >
    > -tkc


    For parameterized decorators, I've usually seen the pattern below.
    Basically, you have 6 lines of boilerplate, and 2 lines of signal.
    The amount of boilerplate is fairly daunting, but I like the
    explicitness, and the nature of decorators is that they tend to get a
    lot of reuse, so you can amortize the pain of all the boilerplate.

    import functools

    def hello_world(name): # non-boilerplate signature
    def decorator(f):
    @functools.wraps(f)
    def wrapped(*args, **kw):
    print 'hello', name # non-boilerplate value-add
    f(*args, **kw)
    return wrapped
    return decorator

    @hello_world('earth')
    def add(x, y):
    print x + y

    add(2, 2)
     
    Steve Howell, Sep 15, 2012
    #5
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Jean-Michel Pichavant

    Re: Decorators not worth the effort

    Jean-Michel Pichavant, Sep 14, 2012, in forum: Python
    Replies:
    1
    Views:
    150
    Steven D'Aprano
    Sep 14, 2012
  2. andrea crotti

    Re: Decorators not worth the effort

    andrea crotti, Sep 14, 2012, in forum: Python
    Replies:
    0
    Views:
    149
    andrea crotti
    Sep 14, 2012
  3. Chris Angelico

    Re: Decorators not worth the effort

    Chris Angelico, Sep 14, 2012, in forum: Python
    Replies:
    2
    Views:
    175
    88888 Dihedral
    Sep 14, 2012
  4. Jean-Michel Pichavant

    Re: Decorators not worth the effort

    Jean-Michel Pichavant, Sep 14, 2012, in forum: Python
    Replies:
    0
    Views:
    158
    Jean-Michel Pichavant
    Sep 14, 2012
  5. andrea crotti

    Re: Decorators not worth the effort

    andrea crotti, Sep 14, 2012, in forum: Python
    Replies:
    0
    Views:
    158
    andrea crotti
    Sep 14, 2012
Loading...

Share This Page