Opened 12 years ago
Closed 5 years ago
#3977 enhancement closed wontfix (wontfix)
Reintegrate qt4reactor into twisted
Reported by: | jknight | Owned by: | Michael |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | core | Keywords: | |
Cc: | ghtdak, Jean-Paul Calderone, Thijs Triemstra | Branch: |
branches/qt4reactor-pyside-3977-2
branch-diff, diff-cov, branch-cov, buildbot |
Author: | michaelnt, thijs |
Description
So, the inevitable has happened, and there are now finally LGPL python bindings for the previously LGPL'd Qt, called "PySide": http://codeposts.blogspot.com/2009/08/lgpl-python-bindings-for-qt-released.html
Supposedly, PySide is even API compatible with PyQt: just replace every PyQt4
import with a PySide
import.
It'd thus be nice to get a qt4reactor like (http://github.com/ghtdak/qtreactor/tree/master) merged back into twisted, but written for PySide instead of PyQt4.
I'd additionally note that if the compatibility is as it is claimed, an end-user who wants to use the designed-for-PySide qtreactor with PyQt4 -- perhaps because they already have PyQt4 installed and do not have PySide installed -- could pretty trivially poke a thing into sys.modules['PySide.QtCore']
first before calling qt4reactor.install(). But of course that's nothing to do with Twisted.
Attachments (5)
Change History (42)
comment:1 follow-up: 26 Changed 12 years ago by
comment:2 follow-up: 4 Changed 11 years ago by
I'm cool with changing to whatever license makes sense. IIRC, the other contributers would include Therve, Itmar Shtull-Trauring and Gabe Rudy
Maintainer: U{Glenn H Tarbox, PhD<mailto:glenn@…>} Previous maintainer: U{Itamar Shtull-Trauring<mailto:twisted@…>} Original port to QT4: U{Gabe Rudy<mailto:rudy@…>}
The current repo is under the launchpad tx project. Not sure if any clones are up at github with fixes we'd want included.
comment:3 Changed 11 years ago by
Owner: | changed from Glyph to ghtdak |
---|
comment:4 Changed 11 years ago by
Replying to ghtdak:
I'm cool with changing to whatever license makes sense. IIRC, the other contributers would include Therve, Itmar Shtull-Trauring and Gabe Rudy
Maintainer: U{Glenn H Tarbox, PhD<mailto:glenn@…>} Previous maintainer: U{Itamar Shtull-Trauring<mailto:twisted@…>} Original port to QT4: U{Gabe Rudy<mailto:rudy@…>}
Following up on this mailing list thread as to the status of the qt4reactor and its licensing:
http://twistedmatrix.com/pipermail/twisted-python/2010-June/022457.html
It would seem PySide has reached the point where it should be a viable alternative to PyQt, without the license complications.
It is still necessary to attain permission to re-release the qt4reactor code under Twisted's MIT license however.
comment:5 Changed 10 years ago by
I've followed up a bit on the thread referenced by sc, and it appears that the qt4reactor has already been relicensed to something very permissive.
See: http://twistedmatrix.com/pipermail/twisted-python/2010-June/022470.html and https://github.com/ghtdak/qtreactor/blob/master/LICENSE
It seems that only the testing requirements outlined by exarkun above still remain for this ticket, before the code living at https://github.com/ghtdak/qtreactor can be merged into Twisted.
comment:6 Changed 10 years ago by
Keywords: | review added |
---|
I've made some updates to the qtreactor at https://github.com/ghtdak/qtreactor and it now passes the complete Twisted test suite with the possible exception of UDP multicast.
It probably needs some more tests, the dpaste link in the email exarkun references can't be viewed anymore.
I've also added a win32event interface so this reactor can be used with serial ports on Windows.
comment:7 Changed 10 years ago by
Cc: | Thijs Triemstra added |
---|
comment:8 Changed 10 years ago by
I created a branch on github that has commits to
- undo the removal of the original qtreactor,
- update qtdemo.py to use Qt4, PySide and a WebKit view rather than plain html text
- update qtreactor with the latest code from qt4reactor
comment:9 Changed 10 years ago by
Owner: | ghtdak deleted |
---|
Added patch that re-integrates the qtreactor and updates the qtdemo app.
Removed the win32event interface as this might be better in a separate ticket.
comment:10 Changed 10 years ago by
A coupe of coding standard issues to be resolved before this lands:
- copyright header should be bumped from 2001-2004 to 2001-2011
- don't add maintainer email addresses to the docstring to prevent spam and outdated info in the source code
- docstrings on 3 lines instead of 2:
""" foo """
comment:11 follow-up: 12 Changed 10 years ago by
Patch update to address review comments from thijs:
- Updated copyright
- Tidied up docstrings
- Removed maintainer names and emails
comment:12 Changed 10 years ago by
Thanks for the updates. I'd like to try this out, but applying it to the trunk throws some errors:
thijs@ubuntu:~/workspaces/opensource/Twisted/trunk$ patch -p0 < qtreactor-3977.patch patching file b/doc/core/examples/qtdemo.py patching file b/twisted/internet/qtreactor.py Hunk #1 FAILED at 1. 1 out of 1 hunk FAILED -- saving rejects to file b/twisted/internet/qtreactor.py.rej patching file b/twisted/internet/test/reactormixins.py Hunk #1 FAILED at 51. 1 out of 1 hunk FAILED -- saving rejects to file b/twisted/internet/test/reactormixins.py.rej The next patch would delete the file a/twisted/plugins/twisted_qtstub.py, which does not exist! Assume -R? [n] Apply anyway? [n] y patching file a/twisted/plugins/twisted_qtstub.py Reversed (or previously applied) patch detected! Assume -R? [n] Apply anyway? [n] Skipping patch. 1 out of 1 hunk ignored -- saving rejects to file a/twisted/plugins/twisted_qtstub.py.rej patching file b/twisted/plugins/twisted_reactors.py Hunk #1 FAILED at 36. 1 out of 1 hunk FAILED -- saving rejects to file b/twisted/plugins/twisted_reactors.py.rej
comment:13 Changed 10 years ago by
The patch came from git so it has to be applied as
patch -p1 < qtreactor-3977.patch
comment:14 Changed 10 years ago by
Updated the patch and checked that it will apply to trunk as
patch -p0 < qtreactor-3977.patch
comment:15 Changed 10 years ago by
Thanks for the updates. I'm using Ubuntu 10.10 maverick and installed pyside 1.0beta4 in the stock ubuntu python2.6 and it runs fine (with some non-related info printed at the console):
thijs@ubuntu:~/workspaces/opensource/Twisted/trunk$ /usr/bin/python2.6 doc/core/examples/qtdemo.py /home/thijs/.gtkrc-2.0:11: Unable to locate image file in pixmap_path: "Arrows/arrow-up_.png" /home/thijs/.gtkrc-2.0:15: Overlay image options specified without filename /home/thijs/.gtkrc-2.0:21: Unable to locate image file in pixmap_path: "Arrows/arrow-up_.png" /home/thijs/.gtkrc-2.0:25: Overlay image options specified without filename
Is it possible to auto-load the twisted site in the webkit panel, because it currently shows a blank page with the url in the addressbar, making it seem that it couldn't be loaded (although it does when you hit enter in the addressbar). See attached screenshot.
Changed 10 years ago by
Attachment: | qtdemo1.png added |
---|
comment:16 Changed 10 years ago by
pyflakes also reports some unused imports:
$ pyflakes qtdemo.py qtdemo.py:13: 'QObject' imported but unused qtdemo.py:13: 'QCoreApplication' imported but unused qtdemo.py:13: 'QSocketNotifier' imported but unused qtdemo.py:13: 'QTimer' imported but unused qtdemo.py:14: 'QEventLoop' imported but unused qtdemo.py:15: 'QtCore' imported but unused
Changed 10 years ago by
Attachment: | qtreactor-3977.patch added |
---|
comment:17 follow-up: 18 Changed 10 years ago by
Updated patch
- removed un-needed imports
- qtdemo now loads default url on start up.
I think the .gtkrc warning are something to do with theming and are presumably wrong on your setup. The demo app doesn't have any icons so we should ignore these warnings.
Changed 10 years ago by
Attachment: | qtdemo2.png added |
---|
comment:18 Changed 10 years ago by
Replying to michaelnt:
I think the .gtkrc warning are something to do with theming and are presumably wrong on your setup. The demo app doesn't have any icons so we should ignore these warnings.
Totally.
Looks good here:
There's a couple of places that have list of reactors (in docstrings and documentation) that would also need an update.
comment:19 follow-up: 25 Changed 10 years ago by
Added a patch that updates the documentation.
I searched for epoll as a way to discover reactor documentation or references in the source code and couldn't find anywhere else that needs updating.
comment:20 Changed 10 years ago by
comment:21 Changed 10 years ago by
Author: | → thijs |
---|---|
Branch: | → branches/qt4reactor-pyside-3977 |
(In [30761]) Branching to 'qt4reactor-pyside-3977'
comment:25 Changed 10 years ago by
Author: | thijs → michaelnt, thijs |
---|---|
Keywords: | review removed |
Owner: | set to Michael |
Replying to michaelnt:
Added a patch that updates the documentation.
I searched for epoll as a way to discover reactor documentation or references in the source code and couldn't find anywhere else that needs updating.
Thanks, I created a branch, applied the tickets and made some cosmetic changes. Could you also add a line to the Reactor Functionality
table in the choosing-reactor howto? I'm not sure how stable it is, but tested on ubuntu.
comment:26 Changed 10 years ago by
Replying to exarkun:
- Get a buildslave set up with PySide installed to run Qt tests
Any chance you could supply a buildslave michaelnt?
comment:27 Changed 10 years ago by
Update on progress.
The Good News is I have set up a buildslave
http://buildbot.twistedmatrix.com/builders/windows7-32-py2.7
The bad news is that
- running trial with the select reactor some of the QtReactor tests fail, but are incorrectly marked as skips by trial.
- Running trial with the qt reactor causes python to crash, and a dialog to get displayed by windows.
I'll keep looking into this.
Changed 10 years ago by
Attachment: | 15759_15758.diff added |
---|
comment:28 Changed 10 years ago by
Keywords: | review added |
---|---|
Owner: | Michael deleted |
All the tests should pass after this diff is applied if run against PySide 1.0 15759_15758.diff
Can someone apply this to the branch.
comment:30 follow-up: 31 Changed 10 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Michael |
Build results at http://buildbot.twistedmatrix.com/builders/windows7-32-py2.7/builds/79
One failure when running the default reactor, test_qt4reactor failing to import twisted_qtstub. When running the complete test suite using the qt reactor python crashes.
If I re-run the individual tests they pass, I suspect a bug in PySide.
comment:31 Changed 9 years ago by
Replying to michaelnt:
Build results at http://buildbot.twistedmatrix.com/builders/windows7-32-py2.7/builds/79
One failure when running the default reactor, test_qt4reactor failing to import twisted_qtstub. When running the complete test suite using the qt reactor python crashes.
That stub was removed in this branch (as part of [attachment:qtreactor-3977.patch in r30827)? Also fails with the qt reactor..
If I re-run the individual tests they pass, I suspect a bug in PySide.
When running this locally the qt reactor python doesn't crash and the test suite completes with errors:
$ uname --all Linux aspiration 3.0.0-16-generic #28-Ubuntu SMP Fri Jan 27 17:44:39 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux $ python -c "from PySide import __version__ as v; print v" 1.1.0 $ trial --reactor=qt twisted ... Ran 6897 tests in 76.736s FAILED (skips=767, expectedFailures=18, failures=13, errors=11, successes=6089)
comment:33 Changed 9 years ago by
Branch: | branches/qt4reactor-pyside-3977 → branches/qt4reactor-pyside-3977-2 |
---|
comment:34 follow-up: 35 Changed 9 years ago by
(In [33610]) Branching to 'qt4reactor-pyside-3977-2'
comment:35 follow-up: 36 Changed 6 years ago by
Replying to thijs:
(In [33610]) Branching to 'qt4reactor-pyside-3977-2'
I've just done an upgrade / reorganization and it looks pretty stable. It passes all 10,00 + tests although whether the skips are correct will need to be vetted.
The QtReactor has been very thoroughly tested with PyQt4, somewhat thoroughly tested with PySide, and not at all on Qt5. However, there is one project using the reactor and Qt5 so I have high confidence I can get it running and reasonably high confidence it will work. How the reactor nails to Twisted would be, essentially, identical.
If there's interest, I could take this ticket on.
comment:36 Changed 6 years ago by
Replying to ghtdak:
Replying to thijs:
(In [33610]) Branching to 'qt4reactor-pyside-3977-2'
I've just done an upgrade / reorganization and it looks pretty stable. It passes all 10,00 + tests although whether the skips are correct will need to be vetted.
The QtReactor has been very thoroughly tested with PyQt4, somewhat thoroughly tested with PySide, and not at all on Qt5. However, there is one project using the reactor and Qt5 so I have high confidence I can get it running and reasonably high confidence it will work. How the reactor nails to Twisted would be, essentially, identical.
If there's interest, I could take this ticket on.
We'd be very much interested. We're using your qtreactor with some PyQt4 apps at the moment, but are of course planning to move to PyQt5. It would be great if the PyQt5 port from https://github.com/nehbit/aether-public/blob/master/qt5reactor.py could be integrated into Twisted.
comment:37 Changed 5 years ago by
Resolution: | → wontfix |
---|---|
Status: | new → closed |
I don't think this belongs in Twisted; Python packaging is now sufficiently good enough that you can pip install from https://pypi.python.org/pypi/qt5reactor and get a QT5 reactor.
Sounds good.
We need to: