Please fix your Python decorators | Hacker News Reader

Please fix your Python decorators

hynek.me

48 pointsedmorley10 comments

Problem: Inspection on wrapped functions is broken.

Solution: Use a slow, third party library.

I'll (personally) stick with the inspection "problem". Changing program behavior at runtime using inspection against function signatures, unless done ridiculously well, is a recipe for creating an unmaintainable disaster.

Granted, I've seen some pretty cool things done with it (for example, injecting a database connection into a handler function if it has a "db" keyword in its signature), but I've also seen the edge cases where it blows up painfully; explicit vs implicit and all that.

The missing intellisense can be a pain, but the docstring is more reliable in most cases (such as the plethora of stdlib functions with "keyword" arguments which aren't actually keyword arguments since they're implemented in C).

"Slow" is not what I read from the benchmarks tho. :) I prefer to avoid edge cases by default and optimize hotspots if necessary. Especially for stuff I publish on PyPI. Likewise I don't care what people do in internal projects. :) It's just frustrating if you install a lib and stuff blows up. :|

Is it just me, or isn't it obvious now that decorators are, at best, an anti-pattern, or worse, a misguided attempt at code obfuscation?

It's not obvious to me. Example use case:

The last time I wrote a bunch of decorators, I was writing python bindings to a set of C++ classes that each had an internal error state that had to be checked after each time a method was called to see if the method failed. If the error state was set, then subsequent method calls for the same object would fail silently, so it was crucial that the error state be checked. There were dozens of methods per class.

I ended up writing a class decorator that would decorate each method with the appropriate method decorator. The method decorator would call the GetErrorCode method, check if nonzero, reset the error state of the object, and then raise an appropriate python exception if necessary.

Major surgery to change the error handling in the called C++ code was not an option.

Can you suggest a better approach?

Define a function to do the same and call that at the end of every method? These don't seem that different to me:

    def doStuff():
        # do stuff
        return checkError(value)

    @checkError
    def doStuff():
        # do stuff
        return value

EDIT: In fact, I think the above succinctly hits on lotsoflumens' point. The latter clearly obfuscates the fact that an error check is performed behind the decorator. The former makes it very clear that something is happening to the return value before it leaves.

I think the antipattern is examining a function's textual arguments and trying to infer things from the names and such. I've had more trouble with that than it has saved me time, between not being able to diagnose things when the magic breaks and the sheer opacity of where arguments are coming from, not to mention all the other quirky little things that go wrong when you push a system that hard.

Just because you can introspect a thing does not mean you must introspect that thing.

I had this problem a couple of days ago. I wanted to use Django' last_modified decorator like this:

    from django.utils.decorators import method_decorator
    from django.views.decorators.http import last_modified
    from rest_framework import viewsets

    class SomeClassBasedView(viewsets.ViewSet):
        def last_modified_time(self, request):
            return cache.get_or_set('key', some_date)
        
        @method_decorator(last_modified(last_modified_time))
        def get_some_object(self):
            # expensive DB operation

Of course it blew up, even when I decorated the last_modified_time with @staticmethod or @classmethod so I had two choices

1. The misleading simple function definition inside the class body, like:

    class SomeClassBasedView(viewsets.ViewSet):
        def last_modified_time(request):
            return cache.get_or_set('key', some_date)
        
        @method_decorator(last_modified(last_modified_time))
        def get_some_object(self):
            # expensive DB operation

The biggest problem with this that it can be confusing for our junior developers (might be even strange for seasoned Pythonistas). Also an IDE (e.g. PyCharm) marks the request argument with a different color making it even more confusing.

2. Define the function outside of the class body (where it doesn't belong...):

    def last_modified_time(request):
        return cache.get_or_set('key', some_date)

    class SomeClassBasedView(viewsets.ViewSet):
        
        @method_decorator(last_modified(last_modified_time))
        def get_some_object(self):
            # expensive DB operation

For what it's worth, the classmethod issue that concerns the author is mostly a non-issue if you apply your decorator before you wrap your function in a classmethod:

  In [1]: def my_decorator(f):
     ...:     def wrapped(*args, **kwargs):
     ...:         print("wrapping %s" % f.__name__)
     ...:         return f(*args, **kwargs)
     ...:     return wrapped
     ...:
  
  In [2]: class Foo:
     ...:     @classmethod
     ...:     @my_decorator
     ...:     def meth(cls, x, y):
     ...:         return x + y
     ...:
  
  In [3]: Foo.meth(1, 2)
  wrapping meth
  Out[3]: 3

This works because the user-defined decorator gets applied to the underlying function before it gets wrapped in a classmethod object (which doesn't supply the desired __name__ attribute).

The signature introspection issue is real, and dealing with it in full generality requires a fair amount of work, especially if you want to be compatible with python 2 and python 3.

In general, classmethod and staticmethod and property should almost always be the last decorator applied if you decorate a method multiple times in Python. This is because, unlike most decorators, classmethod/staticmethod/property don't return functions; they return objects that implement Python's descriptor protocol (https://docs.python.org/3/howto/descriptor.html) slightly differently than how it's implemented by functions. (The core of how methods work in Python is based on the fact that functions are just descriptors (https://docs.python.org/3/howto/descriptor.html#functions-an...).

> unintentionally changes the signatures of my callables

Why it must not change signatures? Everyone says that it shouldn't, but why? Maybe because named the same as Java/.Net decorator pattern? Decorators in Python is almost just a function composition and it can change signature.

Reply on news.ycombinator.com