GrahamDumpleton / wrapt

A Python module for decorators, wrappers and monkey patching.
BSD 2-Clause "Simplified" License
2.03k stars 231 forks source link

Accessing a class attribute that is a wrapt wrapped function will try and bind the function. #256

Open lcrh opened 9 months ago

lcrh commented 9 months ago

Simple reproduction:

import wrapt

@wrapt.decorator
def passthrough(wrapped, instance, args, kwargs):
  return wrapped(*args, **kwargs)

@passthrough
def foo():
  print('foo')

class A:
  _foo = foo

print(type(A._foo))
A._foo()

This returns:

<class 'BoundFunctionWrapper'>
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
[... noisy traceback]
TypeError: missing 1 required positional argument

If a bogus argument is supplied, then the error occurs when the underlying wrapped function is called.

A._foo(1)

leads to

TypeError: foo() takes 0 positional arguments but 1 was given

A possible workaround is to store the function as as singleton list:

import wrapt

@wrapt.decorator
def passthrough(wrapped, instance, args, kwargs):
  return wrapped(*args, **kwargs)

@passthrough
def foo():
  print('foo')

class A:
  _foo = [foo]

print(type(A._foo[0]))
A._foo[0]()

Which yields:

<class 'FunctionWrapper'>
foo

As far as I can tell the failing code is the check at line 666 😈 in wrappers.py: image

lcrh commented 9 months ago

By the way, I really appreciate the work that went into this. wrapt has been invaluable for building enact. We use it for wrapping python code to perform automatic telemetry and replays, e.g., to search the space of program executions of a non-deterministic python program, which comes in handy in the context of working with LLMs.

GrahamDumpleton commented 9 months ago

Can you explain better why it is a problem for you and what you are trying to do?

Since you are using () and triggering a call the behaviour is as I would expect, including the error.

If you want the wrapped function to be tolerant of bogus arguments you should use:

@passthrough
def foo(*args, **kwargs):
  print('foo')

or filter them out in passthrough decorator wrapper function when calling the wrapped function.

@wrapt.decorator
def passthrough(wrapped, instance, args, kwargs):
  # Ignore all arguments.
  return wrapped()
lcrh commented 9 months ago

It's not the behavior that I expected, since I assumed that a decorator such as passthrough would not change the semantics of the function it wraps. My use case is related to telemetry, so maybe let's take logging as an example.

import logging

@wrapt.decorator
def log_calls(wrapped, instance, args, kwargs):
  logging.info('%s called with %s / %s', wrapped, args, kwargs)  
  return wrapped(*args, **kwargs)

I would have assumed that the intended contract is that annotating any function in a python program with log_calls does not change the behavior of the program, except for the additional logging.

It appears this is not the case.

E.g.:

def foo():
  print('foo')

def my_code(fun):
  class A:
    _fun = fun
  A._fun()

my_code(foo)  # succeeds for unwrapped functio
my_code(log_calls(foo))  # fails for wrapped function
GrahamDumpleton commented 9 months ago

Okay, am overlooking that you pointed out error arises from internal to wrapt and not when called by the wrapper.

Let me think about it and I'll do some tests.