Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > Python > Re: Decorators not worth the effort

Reply
Thread Tools

Re: Decorators not worth the effort

 
 
Jean-Michel Pichavant
Guest
Posts: n/a
 
      09-14-2012
----- Original Message -----
> On Sep 14, 3:54*am, Jean-Michel Pichavant <(E-Mail Removed)>
> 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 ) 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
 
Reply With Quote
 
 
 
 
Ulrich Eckhardt
Guest
Posts: n/a
 
      09-14-2012
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

 
Reply With Quote
 
 
 
 
Steven D'Aprano
Guest
Posts: n/a
 
      09-14-2012
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%27t_gonna_need_it


--
Steven
 
Reply With Quote
 
Tim Chase
Guest
Posts: n/a
 
      09-14-2012
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



 
Reply With Quote
 
Steve Howell
Guest
Posts: n/a
 
      09-15-2012
On Sep 14, 6:05*am, Tim Chase <(E-Mail Removed)> 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)
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Re: Decorators not worth the effort Chris Angelico Python 2 09-14-2012 04:37 PM
Re: Decorators not worth the effort andrea crotti Python 0 09-14-2012 04:15 PM
Re: Decorators not worth the effort Jean-Michel Pichavant Python 0 09-14-2012 03:35 PM
Re: Decorators not worth the effort Jean-Michel Pichavant Python 1 09-14-2012 03:19 PM
Re: Decorators not worth the effort andrea crotti Python 0 09-14-2012 02:12 PM



Advertisments