Skip to content
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

Pyside support and usage #114

Open
hellantos opened this issue Aug 29, 2022 · 6 comments
Open

Pyside support and usage #114

hellantos opened this issue Aug 29, 2022 · 6 comments
Labels
help wanted Extra attention is needed

Comments

@hellantos
Copy link
Contributor

@sloretz

Motivation

I was working on ros2/rviz#889 and I have found a couple of issues with qt python integration, especially dependencies. I was looking into creating python bindings for rviz only with PySide2. No I was wondering how to resolve dependencies and turns out there is no key for pyside2 is not in rosdistro but partially in python_qt5_bindings. This all seems kind of unorganised and super hard if you want to create bindings that only support one of the to binding generators.

Problems

pyside2 in rosdistro:
The most essential packages python3-pyside2.qtcore, python3-pyside2.qtgui and python3-pyside2.qtwidgets are not installed by libpyside2-dev, libshiboken2-dev or shiboken in rosdistro. python-pyside2 does not even exist in Ubuntu.

python-qt5-bindings:
  arch: [python2-pyqt5]
  debian:
    buster: [pyqt5-dev, python-pyqt5, python-pyqt5.qtsvg, python-sip-dev, qtbase5-dev]
    jessie: [pyqt5-dev, python-pyqt5, python-pyqt5.qtsvg, python-sip-dev, qtbase5-dev]
    stretch: [pyqt5-dev, python-pyqt5, python-pyqt5.qtsvg, python-sip-dev, qtbase5-dev]
  fedora: [python-qt5-devel, sip]
  freebsd: [py27-qt5]
  gentoo: ['dev-python/PyQt5[gui,widgets]']
  nixos: [pythonPackages.pyqt5]
  openembedded: ['${PYTHON_PN}-pyqt5@meta-qt5']
  opensuse: [python2-qt5-devel, python2-sip-devel]
  slackware: [PyQt5]
  ubuntu:
    artful: [pyqt5-dev, python-pyqt5, python-pyqt5.qtsvg, python-sip-dev]
    bionic: [pyqt5-dev, python-pyqt5, python-pyqt5.qtsvg, python-sip-dev]
    wily: [libpyside2-dev, libshiboken2-dev, pyqt5-dev, python-pyqt5, python-pyqt5.qtsvg, python-pyside2, python-sip-dev, shiboken2]
    xenial: [libpyside2-dev, libshiboken2-dev, pyqt5-dev, python-pyqt5, python-pyqt5.qtsvg, python-pyside2, python-sip-dev, shiboken2]
    yakkety: [libpyside2-dev, libshiboken2-dev, pyqt5-dev, python-pyqt5, python-pyqt5.qtsvg, python-pyside2, python-sip-dev, shiboken2]
    zesty: [pyqt5-dev, python-pyqt5, python-pyqt5.qtsvg, python-sip-dev]

By changing to the following we would at least install the same libraries for pyqt and pyside on Ubuntu:

[libpyside2-dev, libshiboken2-dev, pyqt5-dev, python-pyqt5, python-pyqt5.qtsvg, python3-pyside.qtcore, python3-pyside.qtdbus, python3-pyside.qtdesigner, python3-pyside.qtgui, python3-pyside.qthelp, python3-pyside.qtnetwork, python3-pyside.qtprintsupport, python3-pyside.qttest, python3-pyside.qtwidgets, python3-pyside.qtxml, python3-pyside.qtsvg, python-sip-dev, shiboken2]

Interoperability of PyQt and PySide:
Additionally, why are we not using QtPy(https://github.com/spyder-ide/qtpy) (MIT LICENSE), which has much better support for interoperability between pyside and pyqt, than just replacing names, especially for edge cases. There are also ubuntu/debian binaries in universe.

Conclusion

Qt for Python support is super shady and not well organised. It is very hard to understand which systemdeps to reference to install what. In addition python_qt_bindings will install both pyside2 and pyqt5. Generally, I would rather like to see an approach similar to the rmw's where we have a default and the option to install and use the the other.

Suggestion

  • Add python-qtpy to rosdep
  • Update python_qt_bindings to use QtPy and depreceate importing via python_qt_bindings
  • Use python_qt_bindings package to distribute the CMAKE files for binding generation
  • Update python-qt5-bindings rosdep key to install only default bindings (Use pyqt5 as default, as this seems to be defacto what is happening right now)
  • Create python-pyqt5 rosdep key
  • Create python-pyside2 rosdep key
  • Create additional rosdep keys for both pyside and pyqt in rosdistro for each extra qt lib (e.g. python-pyqt5.qtsvg and python-pyside2.qtsvg)
@clalancette
Copy link
Contributor

You made a very good analysis of the issue, thanks.

In short, I would absolutely love to clean this up. It just hasn't been high on anyone's priority list. I agree with you that it would be much better to pick one solution here, and make it work. Keep in mind that whatever solution we pick has to satisfy the following constraints:

  1. It has to have packages available in the default repositories on at least Ubuntu 22.04 and RHEL (EPEL). There are some possibilities here to import packages from other places, but we very much discourage this so any solution that relies on this will have to be justified.
  2. Any solution we pick needs to work against Qt5 (which is what we are still using).
  3. Any solution we pick has to work on Windows as well.

If the solutions you outline above meet all of that, then we would be happy to review patches to make the conversion over. This will also simplify your other PR for RViz, as we can use the same solution there.

@clalancette clalancette added the help wanted Extra attention is needed label Aug 29, 2022
@hellantos
Copy link
Contributor Author

hellantos commented Aug 29, 2022

First, thanks for the quick response!

  • It has to have packages available in the default repositories on at least Ubuntu 22.04 and RHEL (EPEL). There are some possibilities here to import packages from other places, but we very much discourage this so any solution that relies on this will have to be justified.

Okay, that RHEL narrows things down to pyqt5 as default option, as there is no PySide2 packages available. QtPy is not available in RHEL package index, but it could be installed via pip.

  • Any solution we pick needs to work against Qt5 (which is what we are still using).

PySide adn pyqt have versions that work with Qt5 and Qt6, qtpy works with qt4-6.

  • Any solution we pick has to work on Windows as well.

All are installable with pip on Windows so it should work - primary target of pyside seems to be infact windows. However, pip installs do not come with cmake, so there would need to be a seperate FindPackage.cmake for pip installs in order to make this packages cmake files work.

Plan of Work:
As the qtpy thing propably needs a little more investigation, justification and convincing, I will first do the following:

  • Create PR for python-pyqt5 rosdep key in rosdistro (Ubuntu + RHEL)
  • Create PR for python-pyside2 rosdep key in rosdistro (Ubuntu)
  • Create PR for python-qtpy rosdep key in rosdistro (Ubuntu, pip)
  • Create PR for this package to depend on newly created python-pyqt5 key
  • Create PR for this package to migrate to qtpy - add some more info about advantages of qtpy over doing things manually.

Information needed:
How to test? Editing this package will affect a number of packages. Errors might not be detected in CI. Is there some documentation on how to best test that the changes didn't break anything? Where to notify developers of the changes?

Here are the python_qt_binding dependants:
https://index.ros.org/p/python_qt_binding/github-ros-visualization-python_qt_binding/#humble-deps

@hellantos
Copy link
Contributor Author

Okay, one correction, qtpy seems to be in EPEL 8 https://packages.fedoraproject.org/pkgs/python-QtPy/python3-QtPy/. So I think we can move along with using qtpy as well.

@sloretz
Copy link
Contributor

sloretz commented Aug 29, 2022

Additionally, why are we not using QtPy(https://github.com/spyder-ide/qtpy) (MIT LICENSE)

This package was carried over from ROS 1. I'm not familiar with QtPy, but I'd guess it wasn't an option back when this package was created. It seems like QtPy and python_qt_binding have the same purpose.

QtPy:

QtPy is a small abstraction layer that lets you write applications using a single API call to either PyQt or PySide.

This package python_qt_binding:

This stack provides Python bindings for Qt. There are two providers: pyside and pyqt. PySide2 is available under the GPL, LGPL and a commercial license. PyQt is released under the GPL.
  • Update python_qt_bindings to use QtPy and depreceate importing via python_qt_bindings

Is it possible to remove python_qt_binding completely and switch downstream packages to QtPy? If so, I'd recommend that instead of modifying this package.

@hellantos
Copy link
Contributor Author

hellantos commented Aug 29, 2022

Is it possible to remove python_qt_binding completely and switch downstream packages to QtPy? If so, I'd recommend that instead of modifying this package.

Yes this should be possible.

I have added a temporary pass through in #115, that uses qtpy and warns users, that import via python_qt_binding is going to be deprecated. QtPy has also the advantage that it will ease potential switching to qt6 in the future...

@hellantos
Copy link
Contributor Author

Alright, what I have done for now is the following:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants