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

MNT: Add support to enums in both in PyQt5 and PySide6 #1145

Closed

Conversation

nstelter-slac
Copy link
Collaborator

@nstelter-slac nstelter-slac commented Dec 20, 2024

this should merge after: #1146

Using the python Enum class is a first step required for later
support of enum macros in PySide6,
see https://doc.qt.io/qtforpython-6/PySide6/QtCore/QEnum.html.

Also these python 'Enums' provide some minor benefits like easier iteration,
and are already compatible with PyQt5 enums.

Also remove our subclassing of enum classes, this isn't needed and not
even allowed now that the enums are python 'Enums'.

Also we switch from 'Q_ENUMS' -> 'Q_ENUM' since its a newer verison
of the enum macro for PyQt5.

We import 'Q_ENUM' directly from PyQt5 b/c its not provided by the qtpy
abstraction layer. But this is fine since seems there is no nice
enum abstraction that works for both PyQt5 and PySide6 anyway. The
new enum macro for PySide6 will be added later, and which we use
will be determined with conditional check of Qt wrapper.
@nstelter-slac nstelter-slac changed the title MNT: Pyside2 macro support MNT: Add support to enums in both in PyQt5 and PySide6 Dec 20, 2024
@nstelter-slac nstelter-slac force-pushed the pyside2_macro_support branch 5 times, most recently from 3403788 to b1e5262 Compare December 20, 2024 16:25
This is messy but necessary since qtpy doesn't seem to provide an abstraction over enums that works
with both pyqt and pyside.
@nstelter-slac nstelter-slac marked this pull request as ready for review December 20, 2024 16:29
@nstelter-slac nstelter-slac linked an issue Dec 20, 2024 that may be closed by this pull request
QT_WRAPPER = ""
if importlib.util.find_spec("PySide6"):
QT_WRAPPER = "pyside6"
elif importlib.util.find_spec("PyQt5"):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are considering only supporting Pyside6 and PyQt5 right? we are not considering supporting PyQt6 as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once pyside6 support is in, we could see how much diff there is between pyqt6 and pyside6. if its minimal (maybe the case since both are qt6), i think having support for pyqt6 could be nice.

@@ -1,5 +1,6 @@
from qtpy.QtWidgets import QActionGroup
from qtpy.QtCore import Signal, Slot, Property, QTimer, Q_ENUMS, QThread
from qtpy.QtCore import Signal, Slot, Property, QTimer, QThread
from PyQt5.QtCore import Q_ENUM
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this work for pyside6?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also would it make it so anyone using pyside6 have to also have PyQt5 installed as well?

Copy link
Collaborator

@YektaY YektaY Jan 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait I read your previous PR and see that a pyside6 enum will be added in the future

from .display_format import DisplayFormat, parse_value_for_display
from pydm.utilities import is_pydm_app, is_qt_designer
from pydm import config
from pydm.widgets.base import only_if_channel_set
from PyQt5.QtCore import Q_ENUM
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question as above

@nstelter-slac
Copy link
Collaborator Author

this is implemented instead in #1154, #1155

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Q_ENUMS is deprecated
2 participants