How to write a valid Decorator class in Python?

Question

I just wrote a class decorator, as shown below, tried to add debugging support for each method in the target class:

import unittest import inspect def Debug(targetCls): for name, func in inspect.getmembers(targetCls, inspect.ismethod): def wrapper(*args, **kwargs): print ("Start debug support for %s.%s()" % (targetCls.__name__, name)); result = func(*args, **kwargs) return result setattr(targetCls, name, wrapper) return targetCls @Debug class MyTestClass: def TestMethod1(self): print 'TestMethod1' def TestMethod2(self): print 'TestMethod2' class Test(unittest.TestCase): def testName(self): for name, func in inspect.getmembers(MyTestClass, inspect.ismethod): print name, func print '~~~~~~~~~~~~~~~~~~~~~~~~~~' testCls = MyTestClass() testCls.TestMethod1() testCls.TestMethod2() if __name__ == "__main__": #import sys;sys.argv = ['', 'Test.testName'] unittest.main() 

Run the code above, the result:

 Finding files... done. Importing test modules ... done. TestMethod1 <unbound method MyTestClass.wrapper> TestMethod2 <unbound method MyTestClass.wrapper> ~~~~~~~~~~~~~~~~~~~~~~~~~~ Start debug support for MyTestClass.TestMethod2() TestMethod2 Start debug support for MyTestClass.TestMethod2() TestMethod2 ---------------------------------------------------------------------- Ran 1 test in 0.004s OK 

You may find that "TestMethod2" is printed twice.

Is there a problem? Do I understand this correctly for decorator in python?

Is there a workaround? BTW, I do not want to add a decorator to all methods in the class.

Answer 1

Consider this loop:

 for name, func in inspect.getmembers(targetCls, inspect.ismethod): def wrapper(*args, **kwargs): print ("Start debug support for %s.%s()" % (targetCls.__name__, name)) 

When wrapper is eventually called, it searches for the value of name . Not finding it in locals (), it searches for it (and finds it) in the extended for-loop scope. But by then the for-loop over, and name refers to the last value in the loop, i.e. TestMethod2 .

So, both times the shell is called, name is evaluated as TestMethod2 .

The solution is to create an extended scope where name bound to the correct value. This can be done using the closure function with default argument values. The default values ​​of the arguments are evaluated and fixed during the definition and are bound to variables with the same name.

 def Debug(targetCls): for name, func in inspect.getmembers(targetCls, inspect.ismethod): def closure(name=name,func=func): def wrapper(*args, **kwargs): print ("Start debug support for %s.%s()" % (targetCls.__name__, name)) result = func(*args, **kwargs) return result return wrapper setattr(targetCls, name, closure()) return targetCls 

In the comments, eryksun offers an even better solution:

 def Debug(targetCls): def closure(name,func): def wrapper(*args, **kwargs): print ("Start debug support for %s.%s()" % (targetCls.__name__, name)); result = func(*args, **kwargs) return result return wrapper for name, func in inspect.getmembers(targetCls, inspect.ismethod): setattr(targetCls, name, closure(name,func)) return targetCls 

Now closure needs to be sorted once. Each call to closure(name,func) creates its own function region with even values ​​for name and func .

Answer 2

The problem is not writing a valid class decorator as such; the class is clearly decorated and does not just throw exceptions, you get the code that you would like to add to the class. So you should look for a mistake in your decorator, not the question of whether you can write a valid decorator.

In this case, the problem is with closures. In your Debug decorator, you run a name and func loop, and for each iteration of the loop you define a wrapper function, which is a closure that has access to the loop variables. The problem is that as soon as a new iteration of the loop begins, the things referenced by the loop variables have changed. But you only ever call any of these wrapper functions after completing the entire loop. Thus, each decorated method finishes calling the last values ​​from the loop: in this case TestMethod2 .

What I would do in this case is to make a method level decorator, but since you do not want to decorate each method explicitly, you then create a class decorator that passes all the methods and passes them to the method decorator. This works because you do not provide the shell access to your loop variable through closure; instead, you pass a reference to the loop variable being referenced in the function (a decorator function that creates and returns a wrapper); once this happens, it will not affect the wrapper function to rebuild the loop variable in the next iteration.

Answer 3

This is a very common problem. You think wrapper is a closure that captures the current func argument, but it is not. If you do not pass the current value of func to the shell, this value will be checked only after the loop, so you will get the last value.

You can do it:

 def Debug(targetCls): def wrap(name,func): # use the current func def wrapper(*args, **kwargs): print ("Start debug support for %s.%s()" % (targetCls.__name__, name)); result = func(*args, **kwargs) return result return wrapper for name, func in inspect.getmembers(targetCls, inspect.ismethod): setattr(targetCls, name, wrap(name, func)) return targetCls