GrahamDumpleton / wrapt

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

PySide6 behaves differently between decorated and non-decorated function #243

Open bersbersbers opened 1 year ago

bersbersbers commented 1 year ago

In this code example, fun behaves different from decorated_fun. For more context, see https://stackoverflow.com/q/76596620/. I have tried a lot of things already to fix this. What works is the FunctionBuilder approach of boltons (basically some exec(compile(def fun(...)))). wrapt, however, is not able to fully emulate fun:

import wrapt
# pip install PySide6-Essentials
from PySide6.QtWidgets import QApplication, QSpinBox

def fun():
    print("This works")

@wrapt.decorator
def my_decorator(wrapped, _instance, args, kwargs):
    return wrapped(*args, **kwargs)

@my_decorator
def decorated_fun():
    print("This fails")

app = QApplication()
box = QSpinBox()
box.valueChanged.connect(fun)
box.valueChanged.connect(decorated_fun)
box.setValue(10)
GrahamDumpleton commented 1 year ago

The comments in the StackOverflow issue suggest to me that PySide is doing something unconventional. The comments claim for the decorator package:

Your decorator actually returns a function that can take any parameter. When that "wrapper" is called, everything is fine: using *args, **kwargs allows it to be successfully called with any argument, including no positional argument or unknown keyworded arguments.

Although a visual inspection of the example code may suggest that, from memory the decorator package should use something like the exec(compile(def fun(...))) method you note for boltons and so it should be overriding what should be returned for the wrapper signature so it matches the wrapped function. The wrapt package doesn't use exec(compile(def fun(...))) method, but monkey patches things in other ways so that the signature is preserved.

Any code inspecting the signature using the standard Python functions in the inspect module should thus see the overridden signature and if it bases things on that result, it should be fine.

If that isn't happening, it suggests to me that PySide is not using the standard Python functions, but is dropping down and using lower level parts of the Python object model, meaning it isn't necessarily going to be friendly to situations where duck typing/monkey patching is employed.

To investigate this further, it would be necessary to identify the specific code in PySide where the introspection is being done to work out what it is doing. I can't see in the StackOverflow comments that anyone has pointed into the PySide code to say where that is done.

GrahamDumpleton commented 1 year ago

Related reading about really weird stuff PySide does.

GrahamDumpleton commented 1 year ago

BTW, try your example again using wrapt, but this time set and export the environment variable WRAPT_DISABLE_EXTENSIONS=true first.

This will cause wrapt to use the pure Python version of the wrapper object. I want to see if PySide behaves differently depending on whether that is used instead of the default C extension version.

GrahamDumpleton commented 1 year ago

Actually, that probably wouldn't make a difference since the signature override code is all implemented in pure Python code. All the same, just check in case.

GrahamDumpleton commented 1 year ago

Few other things which would be helpful.

What Python version are you using? How the signature stuff works varies based on the version of Python used.

It would also be helpful to see what you get if you add:

import inspect

...

print(fun.__signature__)
print(decorated_fun.__signature__)

print(inspect.signature(fun))
print(inspect.signature(decorated_fun))

print(inspect.getfullargspec(fun))
print(inspect.getfullargspec(decorated_fun))
GrahamDumpleton commented 1 year ago

Finally, if PySide doesn't want to play well, you may be able to brute force it to work by using signature adapter function as explained in:

Eg.,

def _decorated_fun_prototype(): pass

@my_decorator(adapter=_decorated_fun_prototype)
def decorated_fun():
    print("This fails")

Would rather we can dig into the underlying problem as to why PySide doesn't like how wrapt attempts to preserve the signature.

bersbersbers commented 1 year ago

Wow, thanks for your comments! I will respond to them one by one:

from memory the decorator package should use something like the exec(compile(def fun(...))) method

You are right, although not by default any more: https://github.com/micheles/decorator/blob/bd49b3d/src/decorator.py#L50 But it has preseved decorator.decoratorx, which does in fact work.

If that isn't happening, it suggests to me that PySide is not using the standard Python functions, but is dropping down and using lower level parts of the Python object model, meaning it isn't necessarily going to be friendly to situations where duck typing/monkey patching is employed.

To investigate this further, it would be necessary to identify the specific code in PySide where the introspection is being done to work out what it is doing. I can't see in the StackOverflow comments that anyone has pointed into the PySide code to say where that is done.

I believe you are right. I have been able to confirm that PyQt6's qpycore_pyqtslot.cpp has a check for PyErr_GivenExceptionMatches(xtype, PyExc_TypeError) and then successively removes arguments: nsa = PyTuple_GetSlice(sa, 0, PyTuple_Size(sa) - 1). That would explain why any kind of signature manipulation is fruitless as long as the wrapper continues to accept *args, **kwargs, and an exec-based approach may be necessary.

I am still looking for the same piece of code in PySide6, but it is very possible it is doing something similar.

BTW, try your example again using wrapt, but this time set and export the environment variable WRAPT_DISABLE_EXTENSIONS=true first.

Done - that does not seem to change anything.

What Python version are you using?

3.11.4 on Windows 22H2

Eg.,

def _decorated_fun_prototype(): pass

@my_decorator(adapter=_decorated_fun_prototype)
def decorated_fun():
    print("This fails")

I may be using this wrong, but I am getting

Traceback (most recent call last):
  File "C:\Code\project\bug3.py", line 20, in <module>
    @my_decorator(adapter=_decorated_fun_prototype)
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\bers\.pyenv-win-venv\envs\project_3.11\Lib\site-packages\wrapt\wrappers.py", line 598, in __call__
    return self._self_wrapper(self.__wrapped__, self._self_instance,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\bers\.pyenv-win-venv\envs\project_3.11\Lib\site-packages\wrapt\decorators.py", line 308, in _wrapper
    target_wrapped = args[0]
                     ~~~~^^^
IndexError: tuple index out of range

I have summarized all my experiments in this single piece of code:

import functools
import inspect

# pip install boltons==23.0.0 decorator==5.1.1 wrapt==1.15.0 PySide6-Essentials==6.5.3
import boltons.funcutils
import decorator
import shiboken6  # needed for `import shibokensupport`
import shibokensupport
import wrapt
from PySide6.QtWidgets import QApplication, QSpinBox

# region Decorator( factorie)s

def dec_functools(wrapped):
    @functools.wraps(wrapped)
    def wrapper(*args, **kwargs):
        return wrapped(*args, **kwargs)

    wrapper.__signature__ = inspect.signature(wrapped)
    return wrapper

@wrapt.decorator
def dec_wrapt(wrapped, _instance, args, kwargs):
    return wrapped(*args, **kwargs)

def _my_adapter_prototype():
    pass

@wrapt.decorator(adapter=_my_adapter_prototype)
def dec_wrapt_adapter(wrapped, _instance, args, kwargs):
    return wrapped(*args, **kwargs)

@decorator.decorator
def dec_decorator(wrapped, *args, **kwargs):
    return wrapped(*args, **kwargs)

@decorator.decoratorx
def dec_decoratorx(wrapped, *args, **kwargs):
    return wrapped(*args, **kwargs)

def dec_boltons(wrapped):
    @boltons.funcutils.wraps(wrapped)
    def wrapper(*args, **kwargs):
        return wrapped(*args, **kwargs)

    wrapper.__signature__ = inspect.signature(wrapped)
    return wrapper

def eval_wraps(wrapped):
    def eval_decorator(wrapper, wrapped=wrapped):
        # Generate
        name = wrapped.__name__
        signature = inspect.signature(wrapped)
        arguments = ", ".join(signature.parameters.keys())
        source = f"def {name}{signature}:\n return wrapper({arguments})"

        # Compile
        code = compile(source, "<string>", "exec")

        # Run
        environment = {"wrapper": wrapper}
        eval(code, environment)
        wrapped = environment[name]
        return wrapped

    return eval_decorator

def dec_eval(wrapped):
    @eval_wraps(wrapped)
    def wrapper(*args, **kwargs):
        return wrapped(*args, **kwargs)

    return wrapper

# endregion

# region Decorated functions

def fun(a=0, b=1, c=2):
    print("Works")

@dec_functools
def fun_functools(a=0, b=1, c=2):
    print("Fails")

@dec_wrapt
def fun_wrapt(a=0, b=1, c=2):
    print("Fails")

@dec_wrapt_adapter
def fun_wrapt_adapter(a=0, b=1, c=2):
    print("Fails")

@dec_decorator
def fun_decorator(a=0, b=1, c=2):
    print("Fails")

@dec_decoratorx
def fun_decoratorx(a=0, b=1, c=2):
    print("Works!")

@dec_boltons
def fun_boltons(a=0, b=1, c=2):
    print("Works!")

@dec_eval
def fun_eval(a=0, b=1, c=2):
    print("Works!")

# endregion

# region Tests

for this_fun in [
    fun,
    fun_functools,
    fun_wrapt,
    fun_wrapt_adapter,
    fun_decorator,
    fun_decoratorx,
    fun_boltons,
    fun_eval,
]:
    print(this_fun.__name__)
    print(getattr(this_fun, "__signature__", None))
    print(inspect.signature(this_fun))
    print(inspect.getfullargspec(this_fun))
    print(this_fun.__code__.co_argcount)
    print(this_fun.__code__.co_varnames)
    print(shibokensupport.signature.get_signature(this_fun))
    print()

app = QApplication()
box = QSpinBox()
box.valueChanged.connect(fun)  # works, but is undecorated
box.valueChanged.connect(fun_functools)  # raises TypeError without `a=0, b=1, c=2`
box.valueChanged.connect(fun_wrapt)  # raises TypeError without `a=0, b=1, c=2`
box.valueChanged.connect(fun_wrapt_adapter)  # raises TypeError without `a=0, b=1, c=2`
box.valueChanged.connect(fun_decorator)  # raises TypeError without `a=0, b=1, c=2`
box.valueChanged.connect(fun_decoratorx)  # works
box.valueChanged.connect(fun_boltons)  # works
box.valueChanged.connect(fun_eval)  # works (but uses `eval`)
box.setValue(10)

# endregion

Commented lines produces errors if you remove the a=0, b=1, c=2 part from the function definitions (which I added to investigate if a non-trivial function signature is preserved).

Output:

fun
None
(a=0, b=1, c=2)
FullArgSpec(args=['a', 'b', 'c'], varargs=None, varkw=None, defaults=(0, 1, 2), kwonlyargs=[], kwonlydefaults=None, annotations={})
3
('a', 'b', 'c')
None

fun_functools
(a=0, b=1, c=2)
(a=0, b=1, c=2)
FullArgSpec(args=['a', 'b', 'c'], varargs=None, varkw=None, defaults=(0, 1, 2), kwonlyargs=[], kwonlydefaults=None, annotations={})
0
('args', 'kwargs')
None

fun_wrapt
None
(a=0, b=1, c=2)
FullArgSpec(args=['a', 'b', 'c'], varargs=None, varkw=None, defaults=(0, 1, 2), kwonlyargs=[], kwonlydefaults=None, annotations={})
3
('a', 'b', 'c')
None

fun_wrapt_adapter
()
()
FullArgSpec(args=[], varargs=None, varkw=None, defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
0
()
None

fun_decorator
(a=0, b=1, c=2)
(a=0, b=1, c=2)
FullArgSpec(args=['a', 'b', 'c'], varargs=None, varkw=None, defaults=(0, 1, 2), kwonlyargs=[], kwonlydefaults=None, annotations={})
0
('args', 'kw')
None

fun_decoratorx
None
(a=0, b=1, c=2)
FullArgSpec(args=['a', 'b', 'c'], varargs=None, varkw=None, defaults=(0, 1, 2), kwonlyargs=[], kwonlydefaults=None, annotations={})
3
('a', 'b', 'c')
None

fun_boltons
(a=0, b=1, c=2)
(a=0, b=1, c=2)
FullArgSpec(args=['a', 'b', 'c'], varargs=None, varkw=None, defaults=(0, 1, 2), kwonlyargs=[], kwonlydefaults=None, annotations={})
3
('a', 'b', 'c')
None

fun_my
None
(a=0, b=1, c=2)
FullArgSpec(args=['a', 'b', 'c'], varargs=None, varkw=None, defaults=(0, 1, 2), kwonlyargs=[], kwonlydefaults=None, annotations={})
3
('a', 'b', 'c')
None

Works
Works!
Works!
Works!

Basically, all methods maintain the basic argspec, yet only some of them actually work. Currently, this is fun_decoratorx, fun_boltons, and my minimal fun_my.

GrahamDumpleton commented 1 year ago

My fault on:

def _decorated_fun_prototype(): pass

@my_decorator(adapter=_decorated_fun_prototype)
def decorated_fun():
    print("This fails")

The adapter argument is for wrapt.decorator.

So I meant like example in docs.

def _my_adapter_prototype(arg1, arg2): pass

@wrapt.decorator(adapter=_my_adapter_prototype)
def my_adapter(wrapped, instance, args, kwargs):
    """Adapter documentation."""

    def _execute(arg1, arg2, *_args, **_kwargs):

        # We actually multiply the first two arguments together
        # and pass that in as a single argument. The prototype
        # exposed by the decorator is thus different to that of
        # the wrapped function.

        return wrapped(arg1*arg2, *_args, **_kwargs)

    return _execute(*args, **kwargs)

@my_adapter
def function(arg):
    """Function documentation."""

    pass

I just wasn't paying attention.

So should have been:

def _decorated_fun_prototype(): pass

@wrapt.decorator(adapter=_decorated_fun_prototype)
def my_decorator(wrapped, _instance, args, kwargs):
    return wrapped(*args, **kwargs)

@my_decorator
def decorated_fun():
    print("This fails")

Thinking about it a bit more, if I had to make a guess the problem may be that PySide does equivalent of type() check somewhere along the way and gets confused that the wrapt decorator is actually a class with descriptor binding. It possibly in that case may decide to look at the prototype of the __call__() method of the wrapper object and ignore that the wrapper object instance has a __signature__ attribute. If that is the case, the adapter trick isn't going to work either.

GrahamDumpleton commented 1 year ago

I think what is going to be needed is to work out a standalone test app using PySide to see what it returns when using get_signature() function on a wrapt decorated function.

If that returns bogus results would confirm that it just doesn't cope with decorator wrappers which are implemented as a descriptor object and not a function.

GrahamDumpleton commented 1 year ago

If you add:

from shibokensupport.signature import get_signature

print(get_signature(decorated_fun))

does it work, or does the module import fail.

Am not sure if that is a top level package, or whether is nested in something else and need to work out actual import path.

bersbersbers commented 1 year ago

Inspired by https://github.com/micheles/decorator/issues/129#issue-947497091, I thought I had found the piece of code where PySide6 differs for fun and fun_wrapt:

https://github.com/qtproject/pyside-pyside-setup/blob/483308a17fb8466061f6b318b667758fb5d27d03/sources/pyside6/libpyside/pysidesignal.cpp#L489

https://github.com/qtproject/pyside-pyside-setup/blob/483308a17fb8466061f6b318b667758fb5d27d03/sources/shiboken6/libshiboken/pep384impl.h#L405

However, __code__.co_argcount/__code__.co_varnames are the same for both, so that is probably not the root cause.

I think what is going to be needed is to work out a standalone test app using PySide

We have that, see my initial code

to see what it returns when using get_signature() function on a wrapt decorated function.

It returns None for all. I will update the code example and output in my previous comment immediately.

bersbersbers commented 1 year ago

the adapter trick isn't going to work either

That is indeed the case.

GrahamDumpleton commented 1 year ago

I am indeed blind. If get_signature() returns None, then we are possibly targeting the wrong function. It may just be returning stuff from its cache, do need to find where it populates it cache.

GrahamDumpleton commented 1 year ago

Does get_signature() still return None if given a function which takes arguments? Is that we are using a function with no arguments a bad test case and it perhaps uses None to indicate it doesn't need to do anything special?

I still can't understand how PySide code works. So confusing. :-(

bersbersbers commented 1 year ago

Does get_signature() still return None if given a function which takes arguments?

No, it does not. Good point!

Is that we are using a function with no arguments a bad test case and it perhaps uses None to indicate it doesn't need to do anything special?

Exactly right. I added a, b, c to all signatures and updated https://github.com/GrahamDumpleton/wrapt/issues/243#issuecomment-1617528841.

bersbersbers commented 1 year ago

I am still trying to analyze https://github.com/qtproject/pyside-pyside-setup/blob/dev/sources/pyside6/libpyside/pysidesignal.cpp to see which way they are seeing which signatures are available. They do things that look similar to that in signalInstanceConnect (https://github.com/qtproject/pyside-pyside-setup/blob/72184f8/sources/pyside6/libpyside/pysidesignal.cpp#L451C24-L533) but also signalInstanceEmit (https://github.com/qtproject/pyside-pyside-setup/blob/72184f8/sources/pyside6/libpyside/pysidesignal.cpp#L581-L598).

I just don't understand yet what this ->d thing is - I don't even get a proper type of this thing. It does look like a collection of signatures, but I do not understand what exactly this is (overloads? one for each combination of default parameters?) and where these are created.