Ben
Ben

Reputation: 2472

Issue with monkeypatching methods and references

I was wondering if anyone could explain and offer a solution to this issue:

$ cat object-override-methods.py 
class A:
    def foo(self):
        return 1

class B:
    def foo(self):
        return 1

for klass in A, B:
    orig_foo = klass.foo
    def foo(self):
        return orig_foo(self) * 2
    klass.foo = foo

A().foo()
B().foo()
$ python object-override-methods.py
Traceback (most recent call last):
  File "object-override-methods.py", line 15, in <module>
    A().foo()
  File "object-override-methods.py", line 12, in foo
    return orig_foo(self) * 2
TypeError: unbound method foo() must be called with B instance as first argument (got A instance instead)

Thanks in advance.

Upvotes: 2

Views: 163

Answers (2)

unutbu
unutbu

Reputation: 880927

orig_foo is a global variable which changes value with each pass through the loop. After the loop is done, orig_foo refers to B.foo.

The inner functions foo (one or each pass through the loop) both use the global value for orig_foo when they are called. So they both call B.foo(self).

When calling an "unbound method" like orig_foo, Python2 checks that the first argument is an instance of the appropriate class. A().foo() does not pass this check. (Interestingly, this check was removed in Python3, so there would be no TypeError raised, and this bug may become harder to find.)

To fix this, you must bind the value of orig_foo to the appropriate klass. You can do that by making orig_foo a local variable of foo. One way to do that is to make orig_foo an argument of foo with a default value. Python binds default values at the time a function is defined. So orig_foo=orig_foo binds the local variable orig_foo to the current value of the klass.foo:

for klass in A, B:
    orig_foo = klass.foo
    def foo(self, orig_foo=orig_foo):
        return orig_foo(self) * 2
    klass.foo = foo

Upvotes: 2

RichieHindle
RichieHindle

Reputation: 281875

Because orig_foo is defined at global scope, you're trampling on its value each time round the loop. That trampled value is then shared by each of your new foo methods.

A simple fix is to move the code into a function, like this:

def rebind_foo(klass):
    orig_foo = klass.foo
    def foo(self):
        return orig_foo(self) * 2
    klass.foo = foo

for klass in A, B:
   rebind_foo(klass)

That ensures that each new foo method gets its own value of orig_foo.

Upvotes: 2

Related Questions