When combined with @property methods, `instance` argument does not seem to match the docs
My understanding from the documentation is that for decorated instance methods, you never should have to extract the self arg, that it is always in instance, not args, and that wrapped is already bound to instance. However, this does not seem true when combined with the property decorator:
from pytest import fixture
from wrapt import decorator
@decorator
def wrapper(wrapped, instance, args, kwargs):
return {'instance':instance, 'args':args, 'kwargs': kwargs, 'wrapped':wrapped(*args, **kwargs)}
class Decorated:
@property
@wrapper
def prop(self, *args, **kwargs):
return {'self': self, 'args': args, 'kwargs':kwargs}
@fixture
def instance(): return Decorated()
@fixture
def result(instance): return instance.prop
def test_instance_is_sent_to_wrapped(instance, result): # passes
assert result['wrapped']['self'] is instance
def test_instance_arg_is_correct(instance, result): # fails
assert result['instance'] is instance
def test_instance_is_not_i_args(instance, result): # fails
assert instance not in result['args']
(running on python 3.7.8, wrapt 1.12.1, pytest 5.3.2)
This doesn't seem intentional, and I suspect is @property pulling some shenanigans. If these test don't use @property, and are modified to use a method, they pass. Also, if you grab the property descriptor, calling descriptor.fget requires an instance arg, it isn't bound.
Can this issue be fixed?
I'll try and look later, but property does a lot of weird magic and has conventions that aren't going to work with a standard decorator. You would need to create a very special object wrapper just for property but it could only be applied around it, not under it. Alternatively, you can try and write it long hand, but still not sure how that will work as it is possible that property has same failing classmethod used to, which is that it doesn't honour the descriptor protocol for instance methods it is applied to and uses a short cut.
Anyway, play with the following:
class C:
def __init__(self):
self._x = None
@decorator
def getx(self):
return self._x
@decorator
def setx(self, value):
self._x = value
@decorator
def delx(self):
del self._x
x = property(getx, setx, delx, "I'm the 'x' property.")
If in all cases self is not passed as instance, then property has a similar problem to:
- https://bugs.python.org/issue19072
That only took about 8 years to be fixed and even then they recently think the fix isn't entirely correct.
Would it be possible to normalize this some way, like providing an option to decorator that forces self into args instead of binding? Or is it sufficient to do something like:
if instance is None:
instance = args[0]
normalized_args = args[1:]
…
return wrapped(*args, **kwargs)
It would be easier for you do this yourself for a specific case where it is required. I don't see that having this in wrapt itself would be a good idea.
You haven't really explained the overall goal of what you are trying to achieve and are focusing instead on what you think is the solution.
As I already said you may instead want to create a very special object wrapper just for property which takes into consideration properly the differences between getter, setter and deleter.
So if you can explain properly what it is you are trying to do and why then can perhaps suggest better way of handling it.
Here's an example of my use case:
def with_data_item(data_key):
@decorator
def wrapper(wrapped, instance, args, kwargs):
def adapter(*_args, **_kwargs): # is this adapter needed? I am removing an arg from the decorated signature
data_value = instance.get(data_key)
return wrapped(data_value, *_args, **_kwargs)
return adapter(*args, **kwargs)
return wrapper
class DataAdapter:
def __init__(self, data_source):
self.data_source = data_source
def get(self, data_key):
return self.data_item[data_key]
@property
@with_data_item('foo')
def foo(self, data_item):
return datai_tem+1
@with_data_item('bar')
def calculate_bar(self, data_item):
do_something_slow_with(data_item)
It's similar to functools.partial_method, but I need more flexibility.
Old issue related to this with more thorough description of how to create a wrapper object for properties can be found at:
- https://github.com/GrahamDumpleton/wrapt/issues/44