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

PyQt or 'Qt for Python' (PySide2)? #1875

Closed
ferdnyc opened this issue Jul 19, 2018 · 3 comments
Closed

PyQt or 'Qt for Python' (PySide2)? #1875

ferdnyc opened this issue Jul 19, 2018 · 3 comments
Labels
💡 enhancement This issue describes an improvement, enhancement, or feature request for OpenShot stale This issue has not had any activity in 60 days :(

Comments

@ferdnyc
Copy link
Contributor

ferdnyc commented Jul 19, 2018

In #1864 @DylanC and I were discussing the Timeline implementation and the possible benefits of QML / QT Quick when compared to the current Angular JS.

I realized that there's another, potentially related implementation question looming.

Now that there's an official Qt for Python, based on the old PySide project which the Qt team decided to adopt and support, should OpenShot-qt be written in PySide2 instead of PyQt? And, how much would be involved in porting it over?

On the face of it, there's very little difference between the two. The top of one of their example Python source files looks like this:

from PySide2 import QtCore
from PySide2.QtCore import Qt, QUrl
from PySide2.QtGui import QCloseEvent, QKeySequence, QIcon
from PySide2.QtWidgets import (qApp, QAction, QApplication, QDesktopWidget,
    QDockWidget, QLabel, QLineEdit, QMainWindow, QMenu, QMenuBar, QPushButton,
    QStatusBar, QToolBar)
from PySide2.QtWebEngineWidgets import (QWebEngineDownloadItem, QWebEnginePage,
    QWebEngineView)

Which should seem pretty familiar, compared to our src/windows/views/timeline_webview.py:

from PyQt5.QtCore import QFileInfo, pyqtSlot, QUrl, Qt, QCoreApplication, QTimer
from PyQt5.QtGui import QCursor, QKeySequence
from PyQt5.QtWebKitWidgets import QWebView
from PyQt5.QtWidgets import QMenu

There do appear to be some other differences, though mostly minor.

  • PySide2's signal decorator is @Signal, not @pyqtSignal.
  • PySide2 allows Properties to be declared on Qt objects explicitly, specifying type as well as setter and getter functions. The Property then acts as both a Python property and a Qt property. In PyQt it seems like that's either automatically true for all Python properties, or not supported.
  • If integrating with QML, a Property needs to be NOTIFYable. They give this example of how that works. I'm not sure what PyQt's situation is with that, though I know they also support QML integration.
  • PySide2 loads UI definitions using QUiLoader, via its load() method, which takes an already-opened QFile object. There's no PyQt5.uic, which takes a pathname. That apparently caused some problems when OpenShot is in its frozen state, with the code having to retry the load a few times to find main_window.ui. (See src/classes/ui_util.py at the load_ui() function). So, it's possible PySide2 would solve that for us.

Unfortunately, the Qt Wiki document Differences between PySide and PyQt is over three years old and covers only PySide (not PySide2) vs. PyQt4. And while someone did a comparison of PySide2 vs. PyQt5, with a focus on what PySide2 lacked, that was almost 2 years ago.

While a similar comparison has been periodically posted to the Qt Wiki, updated as recently as a couple of months ago (Qt 5.11), whoever ran the most recent comparison didn't have PyQt5 installed, so there's no info on what PyQt5 has that PySide still lacks. In general, though, most of what's missing appears to be either:

  1. very new;
  2. very, very obscure; or
  3. relegated to a few classes that have major holes, like QtDataVisualization and QtCharts (neither of which exists in PyQt5, though they do offer a separate PyQtChart implementation of QtCharts).

There are some more classes that PySide2 provides which PyQt5 does not, though again we're unlikely to touch those: things like Qt3D{Core,Render,Input,Logic,Animation,Extras} (again, there's a separate PyQt3D for those), or QtConcurrent.

As of the most recent report, there are 219 Qt types missing in PySide2. Which may sound like a lot, as of the previous report (vs. Qt 5.9) there were 163 types missing, and only 41 of those present in PyQt5. So, like I said: Mostly either very new, or very obscure.

@DylanC
Copy link
Collaborator

DylanC commented Jul 23, 2018

@ferdnyc - I did read through this already. Just delayed in my response since we have an overwhelming amount of issues (over 500 now). I will have a much closer look at the PySide2 Technical Preview. Maybe I will just replace the imports and see what happens:

from PySide2 import QtCore
from PySide2.QtCore import Qt, QUrl
from PySide2.QtGui import QCloseEvent, QKeySequence, QIcon
from PySide2.QtWidgets import (qApp, QAction, QApplication, QDesktopWidget,
    QDockWidget, QLabel, QLineEdit, QMainWindow, QMenu, QMenuBar, QPushButton,
    QStatusBar, QToolBar)
from PySide2.QtWebEngineWidgets import (QWebEngineDownloadItem, QWebEnginePage,
    QWebEngineView)

Looks like a minor change with hopefully no real side effects.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Jul 23, 2018

*nod* Other than the import names, it sounds like the decorator swap (@Signal instead of @pyqtSignal) is the major difference that we'd notice, and that would also hopefully amount to little more than a find-and-replace change in the source.

Oh, right, and the resource file(s) — src/images/openshot.qrc at a minimum, and src/language/openshot_lang.qrc if we ever manage to land my translation-loading PR — would have to be recompiled with pyside-rcc, though it sounds like the semantics are the same past that.

PySide also gives us the option to compile the .ui files with pyside-uic... then again, so does PyQt5 which has a pyuic5 — And if loading the .ui files in the frozen state caused such problems (again, see the comments inside load_ui() from src/classes/ui_util.py), I have to wonder why we aren't already doing that, as it would probably eliminate the need for load_ui() and its silliness entirely.

@stale
Copy link

stale bot commented Oct 26, 2020

Thank you so much for submitting an issue to help improve OpenShot Video Editor. We are sorry about this, but this particular issue has gone unnoticed for quite some time. To help keep the OpenShot GitHub Issue Tracker organized and focused, we must ensure that every issue is correctly labelled and triaged, to get the proper attention.
This issue will be closed, as it meets the following criteria: - No activity in the past 180 days - No one is assigned to this issue
We'd like to ask you to help us out and determine whether this issue should be reopened. - If this issue is reporting a bug, please can you attempt to reproduce on the latest daily build to help us to understand whether the bug still needs our attention. - If this issue is proposing a new feature, please can you verify whether the feature proposal is still relevant.
Thanks again for your help!

@stale stale bot added the stale This issue has not had any activity in 60 days :( label Oct 26, 2020
@stale stale bot closed this as completed Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💡 enhancement This issue describes an improvement, enhancement, or feature request for OpenShot stale This issue has not had any activity in 60 days :(
Projects
None yet
Development

No branches or pull requests

4 participants