Modernization: Python 3 and Qt5 #232

Closed
hsoft opened this Issue Feb 18, 2014 · 17 comments

Projects

None yet

8 participants

@hsoft
Member
hsoft commented Feb 18, 2014

I was wondering if there were any Python3 / Qt5 modernization plans for git-cola. If there isn't, I would like to volunteer for that task. There are many paths we can take for that and I wanted to get a discussion started.

My thought is that since git-cola is targeted at developers, there isn't many chances that they run on a system that doesn't have Python 3, so we wouldn't need to keep the code python2-compatible.

Qt5, however, is another story. There hasn't been an Ubuntu LTS release with it yet, so having it as a dependency now might be a bit too soon.

The plan I have in mind is a 3 step modernization process, each of which resulting in a feature-release (1.10, 1.11, 1.12):

  • 1.10: Python 2 modernization (future imports and all) and using PyQt's new api. Drop Python 2.5.
  • 1.11: Python 3
  • 1.12: Qt5

Of course, that's only a proposal I make to get the discussion started. What do you think?

@davvid
Member
davvid commented Feb 18, 2014

I agree that Qt5 is another story. Similarly, it could be argued that we could port to PySide instead of PyQt5, but I don't really have a strong feeling on which way would be best for the future, We'll see where the community goes and we should probably follow suit if there's a clear, obvious winner.

I think Python2 modernization (having a 2+3 compatible codebase) would be a great first step. I've done it on other (not quite as large) code bases with good success, and I imagine that it would probably work out pretty well.

I've already consciously tried to make the internals as amenable to the future as possible -- any data (bytes) that come from the outside goes through a decode step and everything that leaves the internals goes through encode so that the outside sees only bytes. In other words, our internals are already a nice strict unicode sandwich. This should hopefully make supporting 3.x much easier.

I think it would make sense to drop Python2.5 support if we gain 3.x support in the process. I'm imagining that we could probably get pretty far with a 2.6-3.x compatible codebase. Once we're there then we can consider whether it's worth jumping whole hog onto 3.x, but I'd probably be more inclined to keep compatibility if possible since there hopefully won't be much downside.

Thanks for bringing this up; yeah I'm a procrastinator, and python3.x isn't really on our radar at $dayjob yet, so it hasn't been a high priority for me, but it'll need to happen eventually. 👍

@hsoft
Member
hsoft commented Feb 21, 2014

Well then, if you don't mind, I'll get started on python2 modernization + PyQt's new api (the latter probably being the trickiest part).

@hsoft hsoft closed this Feb 21, 2014
@hsoft
Member
hsoft commented Feb 21, 2014

Oops, I misclicked.

@hsoft hsoft reopened this Feb 21, 2014
@davvid
Member
davvid commented Feb 22, 2014

Awesome, thanks. Yeah, the new-style signals+slots is pretty nice. It's probably possible to have pyqt+pyside compatible code using a shim, which would be an interesting experiment, but that's probably a separate topic. Looking forward to it. 👍

@hsoft hsoft referenced this issue Feb 22, 2014
Merged

Support Python 3 #233

@hsoft
Member
hsoft commented Feb 22, 2014

I ended up going straight for Python 3 support and left the "pyqt new api" part for later. I tested the app to the best of my capabilities and I think I've covered a lot of grounds, but I might have missed a few bugs.

Also, the interactive rebasing feature doesn't work for me, even on v1.9.4, so that's a part that I couldn't test at all.

@davvid davvid added a commit to davvid/git-cola that referenced this issue Jan 17, 2016
@davvid davvid pyqt: update to the PyQt4 v2 API for QString and friends
Eliminate all usage of QString and other remnants from the PyQt4 v1 API.
Explicitly set the API to v2 in the new sipcompat module.

Old-style signals and slots are still used.  git-cola will
switch to new-style signals and slots in v3.0.

Related-to: #232
Signed-off-by: David Aguilar <davvid@gmail.com>
2863eb7
@davvid davvid added a commit to davvid/git-cola that referenced this issue Jan 17, 2016
@davvid davvid pyqt: update to the PyQt4 v2 API for QString and friends
Eliminate all usage of QString and other PyQt4 v1 API features.
Explicitly set the API to v2 in the new sipcompat module.

Old-style signals and slots are still used.  git-cola will
switch to new-style signals and slots in v3.0.

Related-to: #232
Signed-off-by: David Aguilar <davvid@gmail.com>
07259cd
@davvid davvid added a commit to davvid/git-cola that referenced this issue Jan 17, 2016
@davvid davvid pyqt: update to the PyQt4 v2 API for QString and friends
Eliminate all usage of QString and other PyQt4 v1 API features.
Explicitly set the API to v2 in the new sipcompat module.

Old-style signals and slots are still used.
git-cola will use new-style signals and slots in v3.0.

Related-to: #232
Signed-off-by: David Aguilar <davvid@gmail.com>
9a04dc5
@davvid
Member
davvid commented Jun 25, 2016

I've started porting to qtpy which seems to be a good way for cola to not care about which library is used, and makes things more flexible for packagers, but I don't see qtpy packaged in fedora (although it is packaged in debian) so I hope it's not a burden on fedora (and other) maintainers to have to package a new dependency.

@comzeradd @kkofler @gcsideal heads up about the upcoming new python dependency. Up until now we've been completely dependency-free.

@hsoft once my port is finished we can close this issue. I suspect this may help #550 so testing from @fny @sitnikovme using PyQt5 and Qt5 would be appreciated once the port is complete, which should be soon.

@kkofler
kkofler commented Jun 26, 2016

To be honest, IMHO, you should just use PyQt5 directly. qtpy is just a pointless wrapper layer that is going to have to run through Fedora review, and in the end will not bring us any added functionality. Qt 4 is obsolescent and some distros are already talking about dropping it (it will remain in Fedora for the foresseable future though, as a deprecated compatibility library). I don't see why a packager would want to make git-cola choose PyQt4 over PyQt5. So in the end, qtpy just makes things more painful for us packagers, as it is yet another dependency to package.

@r-a-v-a-s

Since we are ending the support for Qt 4.8...

http://blog.qt.io/blog/2016/03/16/qt-5-6-released/

@davvid davvid added a commit to davvid/git-cola that referenced this issue Jun 30, 2016
@davvid davvid gravatar: PyQt5 compatibility
QByteArray only accepts byte strings on PyQt5.

Related-to: #232
Signed-off-by: David Aguilar <davvid@gmail.com>
982ad42
@davvid davvid added a commit to davvid/git-cola that referenced this issue Jun 30, 2016
@davvid davvid contrib: one-off pyqt4toqtpypy helper script
Related-to: #232
Signed-off-by: David Aguilar <davvid@gmail.com>
f159b4b
@davvid davvid added a commit to davvid/git-cola that referenced this issue Jun 30, 2016
@davvid davvid search: prepare for PyQt5
Use qtutils.open_files() instead of calling into Qt directly so that we
are protected by upcoming PyQt5-related changes.

Related-to: #232
Signed-off-by: David Aguilar <davvid@gmail.com>
7e0032c
@davvid davvid added a commit to davvid/git-cola that referenced this issue Jun 30, 2016
@davvid davvid widgets: reset() the progress dialog on construction
PyQt5 has a bug where progress dialogs are shown immediately after
construction.  Call reset() to workaround it.

References: https://bugreports.qt.io/browse/QTBUG-47042
Related-to: #232
Signed-off-by: David Aguilar <davvid@gmail.com>
0e0441f
@davvid davvid added a commit to davvid/git-cola that referenced this issue Jun 30, 2016
@davvid davvid doc: mention the Qt5 bugs in the v2.7 release notes
Related-to: #232
Signed-off-by: David Aguilar <davvid@gmail.com>
8bb4915
@davvid
Member
davvid commented Jun 30, 2016 edited

Thanks for the thoughts, @kkofler.

For my purposes, I actually need the wrapper because I have existing users on old platforms that I am not yet prepared to abandon. We will also have a mix of PyQt4 and PyQt5 in production at my $dayjob, for at least another year, so being able to operate in both is a huge advantage for me.

That said, I am sympathetic to packagers and have vendored the QtPy dependency, and added a switch to opt-out of the vendoring, so that packagers are not required to introduce any additional packages. Things will continue to work for existing users as-is.

As for why packagers might choose one over another -- while doing this port I ran into known Qt bugs that cause segfaults on exit and not using the wrapper would have made it impossible for me to A/B test the libraries. As it stands, PyQt4 is the most stable choice and PyQt5 (via Qt5) has bugs that prevent us from recommending it as a stable platform at the moment. I'm optimistic that this will change in the future, though.

I will also note that using QtPy made porting the code much easier because I was able to incrementally port the project one file at a time. Had I not used QtPy then I would have immediately ran into a problem because module A uses PyQt4 while module B uses PyQt5.. so I wouldn't call it completely pointless ;-)

A huge thanks to @ccordoba12 @Nodd @astrofrog and @goanpeca for creating QtPy.

The code is written and is now available in the qtpy branch from my fork https://github.com/davvid/git-cola/tree/qtpy

I'll be testing this for the rest of the week and will likely merge it in by the end of the week or early next week.

OS X users, if you have PyQt5 available (via homebrew?) please test it and let me know if it resolves the "large icons" issue.

@ccordoba12
Contributor

@davvid, hey, this comes as a pleasant surprise to me! git-cola is my favorite git commit tool on Linux, so I'm glad to know it's going to use one of the projects I contribute to ;-)

I recommend you to vendor qtpy 1.1.0 (once we release it) because it has important improvements and bug fixes over 1.0.2.


qtpy is just a pointless wrapper that is going to have to run through Fedora review, and in the end will not bring us any added functionality

Two things here:

  1. We designed qtpy to be used by Spyder, and now it's a hard dependency of its next major version (3.0, not released yet). So I'm sure it's going to be packaged by all major distros eventually, given Spyder's popularity ;-)
  2. It's a not so pointless package because we also have to support PyQt4 and 5, and eventually we hope to add support for PySide2 too.
@goanpeca
Contributor

@davvid, thanks for the kind words, we are very happy to create tools that are useful by other projects :-).

I will also note that using QtPy made porting the code much easier because I was able to incrementally port the project one file at a time. Had I not used QtPy then I would have immediately ran into a problem because module A uses PyQt4 while module B uses PyQt5

This is precisely why we decided to make this package as this road is common for other projects (not only spyder now :-) ). Even though qt4 is deprecated it will take a while for distros and projects to completely migrate.

@ccordoba12
Contributor

@davvid, just a quick note to let you know that we released qtpy 1.1.0 some hours ago.

@fny
fny commented Jul 1, 2016

@davvid I pulled the qtpy repo and ran ./bin/git-cola, but nothing has changed. Git cola may still be using PyQt4. Is there a way for me to check which Qt version is being used or force use of PyQt5?

@davvid
Member
davvid commented Jul 2, 2016

The qtpy docs mention that you can set QT_API to pyqt5 and it will force use of pyqt5. LIkewise pyqt4

@fny
fny commented Jul 2, 2016

@goanpeca It looks like qtpy can't find my PyQt5 installation on OS X.

Even after calling QT_API=pyqt5 ./bin/git-cola, print(qtpy.QtCore.__version__) kept returning v4.8.7, so I uninstalled PyQt4 and tried again to see what would happen, and it doesn't look like qtpy can find the PyQt5 installation. I now receive this error: qtpy.PythonQtError: No Qt bindings could be found.

I've installed PyQt5 using brew install pyqt5, and I can access the library at /usr/local/Cellar/pyqt5/5.6.

Any guidance on how to get this to link seamlessly?

@Nodd
Contributor
Nodd commented Jul 4, 2016

qtpy is just trying import pyqt5, so, you should check that it can be imported in python, and check your PYTHONPATH

@davvid
Member
davvid commented Jul 4, 2016

Googling around, I think you may need to follow the instructions given for sip's caveats:

==> Caveats
The sip-dir for Python is /usr/local/share/sip.
If you need Python to find the installed site-packages:
  mkdir -p ~/Library/Python/2.7/lib/python/site-packages
  echo '/usr/local/lib/python2.7/site-packages' > ~/Library/Python/2.7/lib/python/site-packages/homebrew.pth

Via: http://pastebin.com/Xj1vSJKE

If you do that then python should then be able to find your modules. You can test to see if it's working correctly by doing:

python -c 'import sip'
python -c 'import PyQt5'
@davvid davvid added a commit that closed this issue Jul 11, 2016
@davvid davvid cola: support PyQt5, PyQt4 and Pyside via QtPy
Use the vendored QtPy library to add support for PyQt5 and Pyside.

git-cola is now compatible across PyQt4, PyQt5, and Pyside.
Future QtPy work will add Pyside2 compatibility.

Qt5 contains known bugs that prevent us from recommending it as a stable
target, but this makes it available for users that want to start testing
the waters.

Existing users are not required to install any additional libraries
because we contain a vendored copy of QtPy.  Users that want to use
PyQt5 must install it separately.

Closes #232
Signed-off-by: David Aguilar <davvid@gmail.com>
293364b
@davvid davvid closed this in 293364b Jul 11, 2016
@Nodd Nodd referenced this issue in pyzo/pyzo Aug 18, 2016
Closed

Compatibility with PyQt5 #421

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment