-
-
Notifications
You must be signed in to change notification settings - Fork 306
Refactor Qt brain transforms to handle PyQt and PySide #2863
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Emmanuel Ferdman <[email protected]>
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2863 +/- ##
==========================================
+ Coverage 93.35% 93.40% +0.05%
==========================================
Files 92 92
Lines 11190 11209 +19
==========================================
+ Hits 10446 10470 +24
+ Misses 744 739 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
π New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test matrix for both pyqt 5 and 6 would better work in a separated repository, but we only have an idea of creating astroid plugin repository at this point. Question for other maintainers : This might be the occasion to do a proof of concept for astroid plugin ?
# TODO: enable for Python 3.12 as soon as PyQt6 release is compatible | ||
@pytest.mark.skipif(PY312_PLUS, reason="This test was segfaulting with Python 3.12.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π
- name: Install Qt | ||
if: ${{ matrix.python-version == '3.10' }} | ||
run: | | ||
sudo apt-get update | ||
sudo apt-get install build-essential libgl1-mesa-dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a matrix for pyqt 5 / 6 ?
Type of Changes
Description
This PR rewrites
astroid/brain/brain_qt.py
to correctly infer Qt signals for both PyQt and PySide, regardless of how the binding exposes them: Function-style signals (descriptor) or Class-style signals. Main changes:astroid/brain/brain_qt.py
to share explicit PyQt/PySide signal templates, restoringconnect
/disconnect
/emit
inference for both toolkits. I believe it also improves readability of code.Signal
via the descriptor protocol, so it infers as aFunctionDef
. The transform now catches that path and attaches the expected members. See Example 1 (subclassedQTimer.timeout
) and Example 2 (class-styleSignal()
instance) below. Other bindings (e.g., PyQt5/6) exposepyqtSignal
as a class, so it infers as aClassDef
. We register a matching class transform there as well.Please note that I tried to add tests to
PySide6
but it looks like PyQt6 and PySide6 cannot coexist in a single Linux process because their wheels bundle different Qt 6 library revisions (similiary to concolution in work made in PR #1654). Due to this limiation, I addedpragma: no cover
for thePySide
code branches.Previously failing examples - Example 1:
Used to emit:
Example 2:
Used to emit:
The same pattern affected PyQt5 and PyQt6 as well (with their appropriate API/syntax).
Fixes #2850.