-
Notifications
You must be signed in to change notification settings - Fork 253
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
added PySide6 shiboken6 support #376
Conversation
joshochoa seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Thanks for this. We need tests for this to happen, something to run the code and make sure it works alongside all the other tests but for Qt 6. Can you have a look at https://github.com/mottosso/Qt.py/blob/master/.travis.yml and see how we could get Qt.py running with access to PySide6? We'd also need mention in the README that PySide6 is supported, along with PyQt6. |
@mottosso Any chance we can call in for an assist on that part. |
Calling in all assists! Does anyone feel up to the task of updating our test suite for Qt 6? We're currently on Travis-CI, because in the past it used to be free.. So part of the challenge would be converting it into GitHub Actions. The next challenge is updating our Docker image for the test environment, which is currently very very complicated hahaha. 😅 So a simplified testing environment would be fine, something like a native Ubuntu that GitHub Actions provides natively. Why do all this? Because if you look at virtually every PR to this repository, all of them will have something non-obvious that needs addressing. In particular adding members to Qt.py that shouldn't be there and would break things for someone. It's hard for a single person to test every Python with every Qt binding, especially when introducing 2 more like this PR does. So, it's a challenge, but the hard reality is that it's better to have a limited but working Qt.py than a feature-complete but buggy Qt.py |
https://github.com/mottosso/Qt.py/blob/master/Qt.py#L851 @mottosso but I am far from being an expert around GitHub actions... I made a setup once and I am glad that it still works... |
https://github.com/nebukadhezer/Qt.py/pull/1/files Here I made a small sample PR in my fork.. I placed the tests to run upon merge in master. Idea would be to have a workflows per binding and call the tests per binding... |
It looks (and is) complicated because we're testing against the version of Python and Qt officially part of the VFX Platform, and in that platform there is no Qt 6 yet. Can we test against vanilla Python and vanilla Qt instead..? Probably. But the challenge is finding some way of testing against all versions covered by Qt.py. Because it still needs to support Qt 4 and Qt 5, along with Python 2 and 3. So your approach of adding a GitHub workflow is good, but:
How about adding one of these workflows per version of Qt? That way, you can install an old version of Python and Ubuntu etc. in the workflow, and isolate that environment there. Rather than trying to do what we did before, to run every test for every version in the same workflow. |
hey, sounds good was also what I meant with "one yaml(workflow) per binding"... I am adding them now gradually and with one python variant as a start. But when it comes to unit tests failing I might need some help... And in Qt6 there is a module QtX11Extras gone. I can adapt the unittest to check modules too, but I am not sure within Qt.py how to specify if a module is missing for a specific binding... or we exclude the module in the unitest for Qt6 bindings. Are you still on gitter ? |
The call made at the moment is this one here. https://github.com/mottosso/Qt.py/blob/master/Dockerfile.vfxplatform2018#L807 cp -r /Qt.py /workdir && chmod +x entrypoint.sh && ./entrypoint.sh Which boils down to.. ./entrypoint.sh Which is here. python2.7 -u run_tests.py In addition to
Anything that doesn't exist in all bindings should not be part of Qt.py, so there is no need to test for those modules. They don't exist.
Yes, but it would be best to keep the conversation here since it's a major update. |
QAction is missing |
Linking this conversation about how exactly to incorporate PySide6 - and Qt 6 in general - with Qt.py |
In Qt6 the QAction class, which is used for creation toolbars and menus, has been moved from the QtWidgets to the QtGui module.
The Travis conversion has since been merged (thanks to @martin-chatterjee!), so it should be more feasible to write tests for this one and get the show on the road. 🥳 @zoshua A first step is to merge with latest master, such that your PR also runs in this new test environment. |
adding the QAction to the misplaced dict and removing it from the com…
@mottosso |
Unsure why tests aren't running, so I'll merge this to a temp branch to kick things off. |
There we go, have a look at these broken tests, and submit a new PR please. The new PR should trigger these tests for each commit, making it a bit easier to test. You can also run the tests locally, to speed up iteration time further. |
https://github.com/nebukadhezer/Qt.py/pull/1/files there is still the effort I once put in to run the tests in github actions for pyside6... maybe that is useful to look at. |
Addressed Need for PySide6 and shiboken6 support in Blender 3.3.1