SG-39209 New core patcher for PySide2 to look like PySide6 #1026
Conversation
python/tank/util/qt_importer.py
Outdated
| QT4, QT5, QT6 = range(4, 7) | ||
|
|
||
| def __init__(self, interface_version_requested=QT4): | ||
| def __init__(self, interface_version_requested=QT6): |
There was a problem hiding this comment.
I don't think we want to do that at this point. Otherwise, any code currently using the QtImporter class with the default parameters will break....
There was a problem hiding this comment.
We can add a DeprecationWarning if the interface_version_requestedis QT4.
python/tank/util/qt_importer.py
Outdated
| Tries to import different Qt binding implementation in the following order: | ||
| - PySide2 | ||
| - PySide6 | ||
| - PySide2 |
There was a problem hiding this comment.
I don't see the change in the code reflecting this. Moreover, I think this comment is not accurate because the order depends on the interface_version_requested
| }.get(interface_version_requested) | ||
| logger.debug("Requesting %s-like interface", interface) | ||
|
|
||
| # TODO: Remove this condition sgtk.platform.qt6 is fully supported across all Toolkit repositories. |
There was a problem hiding this comment.
More than a comment, can we raise a DeprecationWarning in that case to start notifying our CI and our community about this project?
python/tank/util/qt_importer.py
Outdated
| module_name = module.name | ||
| for module in pkgutil.iter_modules(PySide6.__path__): | ||
| try: | ||
| module_name = module.name |
There was a problem hiding this comment.
why do we need this change?
docs/platform.rst
Outdated
| it to one of the UI factory methods that the engine has. For example:: | ||
|
|
||
| from sgtk.platform.qt import QtCore, QtGui | ||
| from sgtk.platform.qt import QtCore, QtWidgets |
There was a problem hiding this comment.
| from sgtk.platform.qt import QtCore, QtWidgets | |
| from sgtk.platform.qt6 import QtWidgets |
| for name, cls in QtGui.__dict__.items(): | ||
| if isinstance(cls, type): globals()[name] = cls | ||
|
|
||
| from .qt_abstraction import QtWidgets | ||
| for name, cls in QtWidgets.__dict__.items(): | ||
| if isinstance(cls, type): globals()[name] = cls |
There was a problem hiding this comment.
How was this file generated?
| # other flags. | ||
| self.setWindowFlags(QtCore.Qt.WindowStaysOnTopHint) | ||
| return QtGui.QDialog.exec_(self) | ||
| return QtWidgets.QDialog.exec_(self) |
There was a problem hiding this comment.
What about the exec_ method? I though we were not supposed to use it anymore with recent syntax?
There was a problem hiding this comment.
According to Copilot, on this same line, asking if the exec_ method is deprecated:
No, the exec_ method is not deprecated in PySide6, but the preferred method name is now exec. In PySide6 (and PyQt6), exec_() is still available for backward compatibility, but the trailing underscore was originally used because exec is a reserved keyword in Python 2. Since Python 3, exec is no longer a reserved word, so you can use exec() directly.
So, for PySide6, you should use:
return QtWidgets.QDialog.exec(self)The exec_() method will still work in PySide6 for now, but using exec() is the modern and recommended approach. If you want your code to be future-proof and style-compliant with the latest PySide6 standards, switch to exec().
Summary:
- exec_() is not deprecated yet, but is considered legacy.
- exec() is preferred in PySide6 and PyQt6.
However, reading the documentation and not relying only on AI (I should've done this in the first place to avoid double lookup), I realized that we're encouraged to use open instead (Reference).
There was a problem hiding this comment.
Thanks for the detailed explanation @carlos-villavicencio-adsk!
However, switching from exec_ to open is not straightforward because I am aware that, in some places (TK auth), we rely on exec_ blocking call. So we would have to re-think the workflow to use open.
python/tank/platform/engine.py
Outdated
| :returns: A dictionary with all the modules, __version__ and __name__. | ||
| """ | ||
| return QtImporter(interface_version_requested=QtImporter.QT6).base | ||
| return QtImporter().base |
There was a problem hiding this comment.
No please revert this change
python/tank/platform/engine.py
Outdated
| that emulates Maya 2017's color scheme. | ||
| """ | ||
| from .qt import QtGui | ||
| from .qt import QtGui, QtWidgets |
There was a problem hiding this comment.
| from .qt import QtGui, QtWidgets | |
| from .qt6 import QtGui, QtWidgets |
| """ | ||
|
|
||
| from . import QtCore, QtGui | ||
| from . import QtCore, QtGui, QtWidgets |
There was a problem hiding this comment.
Hum we should import qt6 here right?
Should we move all those modules to the qt6 folder instead?
| # other flags. | ||
| self.setWindowFlags(QtCore.Qt.WindowStaysOnTopHint) | ||
| return QtGui.QDialog.exec_(self) | ||
| return QtWidgets.QDialog.exec_(self) |
There was a problem hiding this comment.
According to Copilot, on this same line, asking if the exec_ method is deprecated:
No, the exec_ method is not deprecated in PySide6, but the preferred method name is now exec. In PySide6 (and PyQt6), exec_() is still available for backward compatibility, but the trailing underscore was originally used because exec is a reserved keyword in Python 2. Since Python 3, exec is no longer a reserved word, so you can use exec() directly.
So, for PySide6, you should use:
return QtWidgets.QDialog.exec(self)The exec_() method will still work in PySide6 for now, but using exec() is the modern and recommended approach. If you want your code to be future-proof and style-compliant with the latest PySide6 standards, switch to exec().
Summary:
- exec_() is not deprecated yet, but is considered legacy.
- exec() is preferred in PySide6 and PyQt6.
However, reading the documentation and not relying only on AI (I should've done this in the first place to avoid double lookup), I realized that we're encouraged to use open instead (Reference).
| @@ -0,0 +1,12 @@ | |||
| class PySide2asPySide6Patcher: | |||
There was a problem hiding this comment.
Let's add the standard Autodesk header on this new file.
| class PySide2asPySide6Patcher: | |
| # Copyright (c) 2025 Shotgun Software Inc. | |
| # | |
| # CONFIDENTIAL AND PROPRIETARY | |
| # | |
| # This work is provided "AS IS" and subject to the Shotgun Pipeline Toolkit | |
| # Source Code License included in this distribution package. See LICENSE. | |
| # By accessing, using, copying or modifying this work you indicate your | |
| # agreement to the Shotgun Pipeline Toolkit Source Code License. All rights | |
| # not expressly granted therein are reserved by Shotgun Software Inc. | |
| class PySide2asPySide6Patcher: |
python/tank/util/qt_importer.py
Outdated
| QT4, QT5, QT6 = range(4, 7) | ||
|
|
||
| def __init__(self, interface_version_requested=QT4): | ||
| def __init__(self, interface_version_requested=QT6): |
There was a problem hiding this comment.
We can add a DeprecationWarning if the interface_version_requestedis QT4.
python/tank/util/qt_importer.py
Outdated
| "QtWebEngineWidgets": QtWebEngineWidgets, | ||
| "QtWidgets": QtWidgets, | ||
| "QtWebEngineCore": QtWebEngineCore, | ||
| "shiboken": shiboken2, |
There was a problem hiding this comment.
If we're mimicking PySide6 in this translator, do we need to change this line to:?
| "shiboken": shiboken2, | |
| "shiboken6": shiboken2, |
I'm not 100% sure about this.
![hound[bot]](https://avatars.githubusercontent.com/in/3598?s=60&v=4){kind=small}
| def qCleanupResources(): | ||
| QtCore.qUnregisterResourceData(0x03, qt_resource_struct, qt_resource_name, qt_resource_data) | ||
|
|
||
| qInitResources() |
There was a problem hiding this comment.
expected 2 blank lines after class or function definition, found 1
| def qInitResources(): | ||
| QtCore.qRegisterResourceData(0x03, qt_resource_struct, qt_resource_name, qt_resource_data) | ||
|
|
||
| def qCleanupResources(): |
| \x00\x00\x01\x8f05\xd2\x9e\ | ||
| " | ||
|
|
||
| def qInitResources(): |
There was a problem hiding this comment.
Black would make changes.
expected 2 blank lines, found 1
| context_info += "You are currently running in the %s environment." % ( | ||
| self._bundle.engine.environment["name"] | ||
| ) | ||
| except: |
There was a problem hiding this comment.
Do not use bare except:, it also catches unexpected events like memory errors, interrupts, system exit, and so on. Prefer except Exception:. If you're sure what you're doing, be explicit and write except BaseException:.
do not use bare 'except'
| # assume that this is derived from an actual tk-multi-workfiles.SaveAsForm! | ||
| cls_type = "SaveAsForm" | ||
|
|
||
| if cls_type != None: |
There was a problem hiding this comment.
comparison to None should be 'if cond is not None:'
| from .. import constants | ||
| from ...errors import TankError | ||
|
|
||
| import sys |
| sizePolicy1.setVerticalStretch(0) | ||
| sizePolicy1.setHeightForWidth(self.details.sizePolicy().hasHeightForWidth()) | ||
| self.details.setSizePolicy(sizePolicy1) | ||
| self.details.setAlignment(Qt.AlignLeading|Qt.AlignLeft|Qt.AlignTop) |
There was a problem hiding this comment.
missing whitespace around bitwise or shift operator
undefined name 'Qt'
|
|
||
| self.details = QLabel(self.frame) | ||
| self.details.setObjectName(u"details") | ||
| sizePolicy1 = QSizePolicy(QSizePolicy.Preferred, QSizePolicy.Expanding) |
|
|
||
| self.verticalLayout.addWidget(self.title) | ||
|
|
||
| self.details = QLabel(self.frame) |
| self.verticalLayout.setObjectName(u"verticalLayout") | ||
| self.title = QLabel(self.frame) | ||
| self.title.setObjectName(u"title") | ||
| sizePolicy = QSizePolicy(QSizePolicy.Expanding, QSizePolicy.Preferred) |
| self.verticalLayout = QVBoxLayout() | ||
| self.verticalLayout.setSpacing(0) | ||
| self.verticalLayout.setObjectName(u"verticalLayout") | ||
| self.title = QLabel(self.frame) |
| self.horizontalLayout_2.setSpacing(2) | ||
| self.horizontalLayout_2.setContentsMargins(2, 2, 2, 2) | ||
| self.horizontalLayout_2.setObjectName(u"horizontalLayout_2") | ||
| self.frame = QFrame(BusyDialog) |
| "font-size: 11px;\n" | ||
| "}\n" | ||
| "") | ||
| self.horizontalLayout_2 = QHBoxLayout(BusyDialog) |
| "margin-bottom: 0px;\n" | ||
| "font-size: 11px;\n" | ||
| "}\n" | ||
| "") |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "margin-left: 3px;\n" | ||
| "margin-bottom: 0px;\n" | ||
| "font-size: 11px;\n" | ||
| "}\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "margin-top: 1px;\n" | ||
| "margin-left: 3px;\n" | ||
| "margin-bottom: 0px;\n" | ||
| "font-size: 11px;\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "#details {\n" | ||
| "margin-top: 1px;\n" | ||
| "margin-left: 3px;\n" | ||
| "margin-bottom: 0px;\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "/* Style for the details text */\n" | ||
| "#details {\n" | ||
| "margin-top: 1px;\n" | ||
| "margin-left: 3px;\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "\n" | ||
| "/* Style for the details text */\n" | ||
| "#details {\n" | ||
| "margin-top: 1px;\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "}\n" | ||
| "\n" | ||
| "/* Style for the details text */\n" | ||
| "#details {\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "font-weight: bold;\n" | ||
| "}\n" | ||
| "\n" | ||
| "/* Style for the details text */\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "margin-bottom: 0px;\n" | ||
| "margin-left: 1px;\n" | ||
| "font-size: 16px;\n" | ||
| "font-weight: bold;\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "margin-top: 15px;\n" | ||
| "margin-bottom: 0px;\n" | ||
| "margin-left: 1px;\n" | ||
| "font-size: 16px;\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "color: #30A7E3;\n" | ||
| "margin-top: 15px;\n" | ||
| "margin-bottom: 0px;\n" | ||
| "margin-left: 1px;\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "#title {\n" | ||
| "color: #30A7E3;\n" | ||
| "margin-top: 15px;\n" | ||
| "margin-bottom: 0px;\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "/* Style for the header text */\n" | ||
| "#title {\n" | ||
| "color: #30A7E3;\n" | ||
| "margin-top: 15px;\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "\n" | ||
| "/* Style for the header text */\n" | ||
| "#title {\n" | ||
| "color: #30A7E3;\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "}\n" | ||
| "\n" | ||
| "/* Style for the header text */\n" | ||
| "#title {\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "border-width: 2px;\n" | ||
| "}\n" | ||
| "\n" | ||
| "/* Style for the header text */\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "border-style: solid;\n" | ||
| "border-width: 2px;\n" | ||
| "}\n" | ||
| "\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
| "border-color: #30A7E3;\n" | ||
| "border-style: solid;\n" | ||
| "border-width: 2px;\n" | ||
| "}\n" |
There was a problem hiding this comment.
continuation line under-indented for visual indent
|
|
||
| from . import QtWidgets | ||
| for name, cls in QtWidgets.__dict__.items(): | ||
| if isinstance(cls, type): globals()[name] = cls |
There was a problem hiding this comment.
multiple statements on one line (colon)
|
|
||
| from . import QtGui | ||
| for name, cls in QtGui.__dict__.items(): | ||
| if isinstance(cls, type): globals()[name] = cls |
There was a problem hiding this comment.
multiple statements on one line (colon)
|
|
||
| from . import QtCore | ||
| for name, cls in QtCore.__dict__.items(): | ||
| if isinstance(cls, type): globals()[name] = cls |
There was a problem hiding this comment.
multiple statements on one line (colon)
| ################################################################################ | ||
|
|
||
| from . import QtCore | ||
| for name, cls in QtCore.__dict__.items(): |
| ## | ||
| ## Created by: Qt User Interface Compiler version 5.15.2 | ||
| ## | ||
| ## WARNING! All changes made in this file will be lost when recompiling UI file! |
There was a problem hiding this comment.
too many leading '#' for block comment
| QtWebEngineWidgets, | ||
| QtWidgets, | ||
| QtWebEngineCore, |
| :param session_metadata: Metadata used in the context of SSO. This is an obscure blob of data. | ||
| """ | ||
| QtGui.QDialog.__init__(self, parent) | ||
| QtWidgets.QDialog.__init__(self, parent) |
There was a problem hiding this comment.
Any reason to not use super()?
|
|
||
| def _confirm_exit(self): | ||
| return self.confirm_box.exec_() == QtGui.QMessageBox.StandardButton.Yes | ||
| return self.confirm_box.exec_() == QtWidgets.QMessageBox.StandardButton.Yes |
There was a problem hiding this comment.
| return self.confirm_box.exec_() == QtWidgets.QMessageBox.StandardButton.Yes | |
| return self.confirm_box.exec() == QtWidgets.QMessageBox.StandardButton.Yes |
python/tank/platform/engine.py
Outdated
| qt_widgets = qt5_base.get("QtWidgets") | ||
| qt5.TankDialogBase = qt_widgets.QDialog if qt_widgets else None |
There was a problem hiding this comment.
Why do we need this change?
|
This PR is too big (lines changed/files affected). Can we split the work into different PRs that don't interfere with each other (e.g. authentication, UI)? And see a way to format files beforehand to avoid Hound messages, they're creating a lot of noise. |
julien-lang
changed the title
| self.app_work_area_info_2.setText(QCoreApplication.translate("TankDialog", u"If you are making changes to configuration or code, use the reload button to quickly load your changes in without having to restart:", None)) | ||
| self.btn_reload.setText(QCoreApplication.translate("TankDialog", u"Reload Engine and Apps", None)) | ||
| self.config_header.setText(QCoreApplication.translate("TankDialog", u"Configuration", None)) | ||
| self.config_label.setText(QCoreApplication.translate("TankDialog", u"Below is a list of all the configuration settings for this app, as defined in your environment file:", None)) |
| self.btn_shotgun.setText(QCoreApplication.translate("TankDialog", u"Jump to Flow Production Tracking", None)) | ||
| self.app_work_area_info_2.setText(QCoreApplication.translate("TankDialog", u"If you are making changes to configuration or code, use the reload button to quickly load your changes in without having to restart:", None)) | ||
| self.btn_reload.setText(QCoreApplication.translate("TankDialog", u"Reload Engine and Apps", None)) | ||
| self.config_header.setText(QCoreApplication.translate("TankDialog", u"Configuration", None)) |
| self.btn_file_system.setText(QCoreApplication.translate("TankDialog", u"Jump to File System", None)) | ||
| self.btn_shotgun.setText(QCoreApplication.translate("TankDialog", u"Jump to Flow Production Tracking", None)) | ||
| self.app_work_area_info_2.setText(QCoreApplication.translate("TankDialog", u"If you are making changes to configuration or code, use the reload button to quickly load your changes in without having to restart:", None)) | ||
| self.btn_reload.setText(QCoreApplication.translate("TankDialog", u"Reload Engine and Apps", None)) |
| self.app_work_area_info.setText(QCoreApplication.translate("TankDialog", u"TextLabel", None)) | ||
| self.btn_file_system.setText(QCoreApplication.translate("TankDialog", u"Jump to File System", None)) | ||
| self.btn_shotgun.setText(QCoreApplication.translate("TankDialog", u"Jump to Flow Production Tracking", None)) | ||
| self.app_work_area_info_2.setText(QCoreApplication.translate("TankDialog", u"If you are making changes to configuration or code, use the reload button to quickly load your changes in without having to restart:", None)) |
| self.label_5.setText(QCoreApplication.translate("TankDialog", u"Your Current Work Area", None)) | ||
| self.app_work_area_info.setText(QCoreApplication.translate("TankDialog", u"TextLabel", None)) | ||
| self.btn_file_system.setText(QCoreApplication.translate("TankDialog", u"Jump to File System", None)) | ||
| self.btn_shotgun.setText(QCoreApplication.translate("TankDialog", u"Jump to Flow Production Tracking", None)) |
| self.gradient.setTitle("") | ||
| self.app_icon.setText("") | ||
| self.app_name.setText(QCoreApplication.translate("TankDialog", u"Publish And Snapshot", None)) | ||
| self.app_description.setText(QCoreApplication.translate("TankDialog", u"Tools to see what is out of date in your scene etc etc.", None)) |
| self.label_3.setText("") | ||
| self.gradient.setTitle("") | ||
| self.app_icon.setText("") | ||
| self.app_name.setText(QCoreApplication.translate("TankDialog", u"Publish And Snapshot", None)) |
| self.details_show.setText("") | ||
| #if QT_CONFIG(tooltip) | ||
| self.details_hide.setToolTip(QCoreApplication.translate("TankDialog", u"Hide App Details", None)) | ||
| #endif // QT_CONFIG(tooltip) |
There was a problem hiding this comment.
block comment should start with '# '
| #endif // QT_CONFIG(tooltip) | ||
| self.details_show.setText("") | ||
| #if QT_CONFIG(tooltip) | ||
| self.details_hide.setToolTip(QCoreApplication.translate("TankDialog", u"Hide App Details", None)) |
| self.details_show.setToolTip(QCoreApplication.translate("TankDialog", u"Click for App Details", None)) | ||
| #endif // QT_CONFIG(tooltip) | ||
| self.details_show.setText("") | ||
| #if QT_CONFIG(tooltip) |
There was a problem hiding this comment.
block comment should start with '# '
eduardoChaucaGallegos
changed the title
3 participants
Description
Tests