search

pyapp-kit / superqt

Missing widgets and components for Qt-python
https://pyapp-kit.github.io/superqt/
BSD 3-Clause "New" or "Revised" License
210 stars 38 forks source link

fix: fix parameter inspection on ensure_thread decorators #183

Closed tlambert03 closed 1 year ago

tlambert03 commented 1 year ago

fixes #182 ... and adds a test, but it sure isn't pretty at the moment :joy:

@Czaki, I'll keep playing with this (and digging into the qt source code) to see if I can come up with a more elegant way for Qt to be able to properly inspect the signature of a decorated wrapped function. but if you have any other ideas i should try, feel free to propose!

edit see also #185, which I think prefer now

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 77.77% and project coverage change: +0.02% :tada:

Comparison is base (dd9af3b) 86.03% compared to head (2fc318f) 86.06%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #183 +/- ## ========================================== + Coverage 86.03% 86.06% +0.02% ========================================== Files 35 35 Lines 2722 2726 +4 ========================================== + Hits 2342 2346 +4 Misses 380 380 ``` | [Files Changed](https://app.codecov.io/gh/pyapp-kit/superqt/pull/183?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyapp-kit) | Coverage Δ | | |---|---|---| | [src/superqt/utils/\_ensure\_thread.py](https://app.codecov.io/gh/pyapp-kit/superqt/pull/183?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyapp-kit#diff-c3JjL3N1cGVycXQvdXRpbHMvX2Vuc3VyZV90aHJlYWQucHk=) | `78.00% <77.77%> (+1.91%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tlambert03 commented 1 year ago

the crux of the pyside code is here: https://github.com/pyside/pyside-setup/blob/10941bc37450f2fe99cd3afa4e2331df0439d162/sources/pyside6/libpyside/pysidesignal.cpp#L1209C2-L1213C20

basically, it looks at .__code__.co_argcount and other __code__ attributes. I tried patching those directly (without using compile and exec), but wasn't able to do so. In any case, I'm not sure it would be any less black magic than the fix here.

I do think this is important functionality to retain though (the ability to add more signals later without breaking APIs is a very nice thing in Qt/psygnal, and this is currently breaking it)

thoughts?

tlambert03 commented 1 year ago

linking https://github.com/pyapp-kit/psygnal/pull/228 here as well, since I suspect a similar phenomenon might be happening in psygnal too. basically, this will likely happen with any decorator that uses the pattern:

@wraps(func)
def inner(*args, **kwargs):
    func(*args, **kwargs)

since Qt doesn't know that inner has the same signature as func