PR: Add QEnum macro for PyQt bindings

[phil65]

synchronizes macro naming with PySide

[CAM-Gerlach]

Thanks. Seems like Q_ENUM doesn't exist on PyQt 5.9, though, which we still support (due to Anaconda previously having been stuck on it until very recently). Is there an alternate name from which it can be imported on that version? Otherwise, you'll presumably want to handle it not being present (e.g. with getattr).

@dalthviz , thoughts?

[phil65]

According to docs, it should have been added to Qt5.5 already. (https://doc.qt.io/qt-5/whatsnew55.html)
Perhaps there exists a Q_ENUMS macro instead of Q_ENUM for older python bindings?

[phil65]

Tried adding a fallback for 5.9.

EDIT: checks are passing now.

[CAM-Gerlach]

LGTM from my side; thanks @phil65 . @dalthviz ?

[dalthviz]

Thank you for the work here @phil65 ! Could it be possible to add some tests for this?

Other than that this LGTM 👍

[CAM-Gerlach]

To note, at least judging by what we've done in the past, it even can be pretty simple—even just an existence check on QEnum (e.g. assert QtCore.QEnum is not None). See the other tests for examples.

[dalthviz]

Note: The errors in the tests come from pypa/pip#11975 and are unrelated to the changes done here

[dalthviz]

Note: Re-running the CI checks shows that there is an error with PySide2 (AttributeError: module 'qtpy.QtCore' has no attribute 'QEnum') 🤔

[phil65]

Will take a look again when I find some time.

[flowln]

Just a note here in case it helps, the QEnum macro was not added in PySide2 until 5.15 (https://bugreports.qt.io/browse/PYSIDE-957), so the tests will always fail on older Qt versions! :c

[dalthviz]

Thank you @flowln for the feedback!

@phil65 I think what is missing then is adding a skip to the new test when PySide2 is being tested and has a version <5.15. Let us know if you need help with that!

[dalthviz]

Hi @phil65 sorry for the sudden ping, just wondering, is it okay if I try to finish this? Let us know!

[dalthviz]

I will try to finish this so please @phil65 don't push any other commit to your branch :)

[ccordoba12]

Looks good to me, thanks @phil65!

[dalthviz]

Is it okay if we merge this one @CAM-Gerlach ?

[CAM-Gerlach]

Checking the very minor changes since my last approval, they all LGTM, thanks—sorry for the delay!