Add pyqt5 support to load_stylesheet()

[matthew-reynolds]

#104 updated qdarkstyle.load_stylesheet() to support both PySide and PySide2. It added a try/except so that whichever version of PySide is available will be selected and used.

This PR adds similar behaviour for detecting & selecting PyQt versions, either PyQt4 or PyQt5. This mean calling qdarkstyle.load_stylesheet(pyside=False) will select whichever version of PyQt is available, whereas before this function only supported PyQt4.

Effectively, the behaviour of load_stylesheet_pyqt5() has been merged into load_stylesheet().

This change also gets qdarkstyle.load_stylesheet_pyqt5() ready for deprecation, matching its siblings qdarkstyle.load_stylesheet_pyqt4(), load_stylesheet_pyside(), and load_stylesheet_pyside2().

[dpizetta]

I'll check carefully this PR. Answer you soon. I'm not sure if I understand what this PR means. Can you write a more detailed description? Tks

[matthew-reynolds]

Updated description and rebased.

Let me know if there's anything I can further clarify :)

[dpizetta]

Hi @matthew-reynolds , now I understand the proposal. Actually, we are moving to insert qtpy depedency, so no more tests of each one are needed since qtpy is an abstraction for both. This is already implemented in version 3, but I'm creating PR for this right now, since it will simplify many scripts and functions we have right now. Sorry for the delayed response and thanks a lot for your contributions. Few free to send new PR's.

[matthew-reynolds]

@dpizetta Thanks for replying. I understand that the intent is to move to qtpy for version 3, but since version 3 is not yet released and this is a step towards improving the deprecation process, I still believe this is a worthwhile PR in the interim until version 3 with the qtpy dependency is implemented.

To elaborate, load_stylesheet_pyqt5() is marked as being deprecated in version 3. However, there is no alternative option to start to future-proof existing code, since load_stylesheet() doesn't support pyqt5. There's currently no way to use a single unified function to load the stylesheet across all 4 supported backends. This PR will make the migration from v2 to v3 more closely aligned with recommended deprecation practices such as SemVar.

TL;DR - I believe the migration path should be the following, rather than skipping straight from 1 to 3:

1. load_stylsheet_pyqt5() # Currently, in version 2.x
2. load_stylesheet(False) # Future, version 2.y
3. load_stylesheet()      # Future, version 3, with qtpy dependency

---

Looking at usage, here is what is currently required for backend-agnostic stylesheet loading:

if 'pyqt' in QT_BINDING:
    if int(QT_BINDING_VERSION[0]) > 5:
        setStyleSheet(qdarkstyle.load_stylesheet_pyqt5())
    else:
        setStyleSheet(qdarkstyle.load_stylesheet_pyqt())
else:
    setStyleSheet(qdarkstyle.load_stylesheet(True))

With this PR, this is changed to a single line:

setStyleSheet(qdarkstyle.load_stylesheet('pyside' in QT_BINDING))

And with the eventual release of v3:

setStyleSheet(qdarkstyle.load_stylesheet())

Clearly, from the above example, this PR simplifies the migration path from v2 to v3 and improves compatibility between versions.

[dpizetta]

Hi @matthew-reynolds, you have raised good points. I'll reopen this PR and we can continue to discuss some other points until the v2.7 is released, so we can add those changes. Tks

[matthew-reynolds]

Thanks for reconsidering @dpizetta :) Looking forward to 2.7 and discussing the best way to get these changes included in a way you're satisfied with.

[dpizetta]

Hi @matthew-reynolds, after inserting the qtpy dependency in v2.8 we have shorted the path to your PR by this code. Could you confirm if it is the desired result as your PR mention?

@goanpeca and @ccordoba12 what do you think about it too?

And about this:

1. load_stylsheet_pyqt5() # Currently, in version 2.x
2. load_stylesheet(False) # Future, version 2.y
3. load_stylesheet() # Future, version 3, with qtpy dependency

It still in discussion, we can keep those calls, for all the next versions as implemented now

• load_stylsheet_pyqt5()
• load_stylsheet_pyqt4()
• load_stylsheet_pyside()
• load_stylsheet_pyside2()

Then, in v2.9 change just the 'load_stylesheet(pyside=False)' to 'load_stylesheet(qt_api='pyqt5')'. So, we keep the same API mainly.

And about this one:

if 'pyqt' in QT_BINDING:
    if int(QT_BINDING_VERSION[0]) > 5:
        setStyleSheet(qdarkstyle.load_stylesheet_pyqt5())
    else:
        setStyleSheet(qdarkstyle.load_stylesheet_pyqt())
else:
    setStyleSheet(qdarkstyle.load_stylesheet(True))

This automatically changes the binding could be done after the PR#156 of QtPy being accepted (I'm revising right now), that will automatically 'rotate' and set an installed binding if the previous set was not found.

Tks

[matthew-reynolds]

@dpizetta Looks good to me! Now that you've introduced the QtPy dependency in v2.8, your implementation in the develop branch achieves everything this PR intended. Once you land v2.8 let's go ahead and close this PR as it will be redundant.

I'm a little bit hesitant about introducing an API-breaking change between 2.8 and 2.9 without following a 'tick-tock' deprecation cycle. Ideally, the changes would be introduced like this such that there is always at least one release with the function marked "Deprecated" before it is completely removed/changed. This way downstream users have one release to catch up to the API without their code breaking. Something like this would be ideal:

Version 2.7:

load_stylsheet_pyqt5()

Version 2.8:

load_stylsheet_pyqt5() # Deprecation warning: "Prefer load_stylesheet(False)"
load_stylesheet(pyside=False)

Version 2.9:

load_stylsheet_pyqt5() # Either deprecated or removed
load_stylesheet(pyside=False) # Deprecation warning: "Prefer load_stylesheet('pyqt5')"
load_stylesheet(qt_api='pyqt5')

Version 2.10 or 3.0 or something future:

load_stylsheet_pyqt5() # Either deprecated or removed
load_stylesheet(pyside=False) # Either deprecated or removed
load_stylesheet(qt_api='pyqt5')

I believe with your current plan for v2.9 wouldn't include load_stylesheet(pyside=False). I think that it's important to preserve this, in deprecated form, for at least one release before removing it. Could be implemented by checking the type of the passed parameter of the load_stylesheet() function.

Ideally, according to semver, the deprecated APIs should remain in place until the next major release at which point they may be removed. Since this project doesn't use semantic versioning, it may be reasonable to remove the APIs in the next minor release, not my place to make that call. However, since we're introducing breaking changes, I do think it's important that there is at least one release with overlap of deprecated and new APIs.

[ccordoba12]

what do you think about it too?

I agree with this. The new code look very simple and manageable in comparison to the multiple checks that were necessary before.

[goanpeca]

It still in discussion, we can keep those calls, for all the next versions as implemented

@dpizetta I believe we should keep them until we move to 3 since the readme is showing the use of those calls in all the examples and yes it would be. Breaking change.

We need to print deprecation warnings that they will be removed in 3 (but i think those are in place already?)

[dpizetta]

Thank you all for the ideas. This is what I have in mind, then

Version 2.7:

# keep those forever
load_stylsheet_pyqt4()
load_stylsheet_pyqt5()
load_stylsheet_pyside()
load_stylsheet_pyside2()

# we will change these
load_stylesheet(pyside=False)
load_stylsheet_from_environment(pyqtgraph=False)

Version 2.8:

load_stylesheet(pyside=True)                              
# Deprecation warning: Prefer load_stylesheet('pyside2')

load_stylsheet_from_environment(pyqtgraph=True)
# Deprecation warning: Prefer load_stylesheet(os.environ['PYQTGRAPH_QT_LIB'])

They still can use the old and the new API. For me it is strange we suggest to use an argument that in the next version (minor/major) will be removed or it is planned to be. So we can keep the users using both API until the version 3. Also, keep those directly calls (_pyqt5, pyside2) to simplify and not create problems with users that already use it.

I've already pushed this in branch develop.

Version 3.0 or something future:

load_stylesheet(pyside=False) # Either deprecated or removed
load_stylesheet(qt_api='pyqt5')

Do you agree? @matthew-reynolds Tks

[matthew-reynolds]

@dpizetta This is getting a bit confusing haha. I believe the weirdness you pointed out about suggesting to use something that is planned to be removed is just the result of trying to move slowly and incrementally.

At the end of the day, I think it's best to just consider the rationale for these deprecations: Provide a notice that this function is soon going to change or disappear. Provide devs with the prefered alternative, and ensure there's overlap between adding the deprecation message and adding the prefered alternative so developers have time to update their code.

After at least one release with the deprecation notice and the alternative available, the deprecated functionality can be removed.

All that to say: I mostly agree with what you posted, although I think that either:

• `load_stylesheet(qt_api) should be included in 2.8 OR
• The deprecation warning on load_stylesheet(pyside) should be included in 3.0 (Rather than totally removed).

In either case, again it comes down to having a bit of overlap between deprecation and new APIs. Let me know what you think and if you agree :)

[dpizetta]

@matthew-reynolds tks for your time! I've forgotten about "`load_stylesheet(qt_api) should be included in 2.8 OR". It is already there, but I was not clear. So, that is, I think hehehe, the final plan.

Version 2.7 - current:

load_stylsheet_pyqt4()
load_stylsheet_pyqt5()
load_stylsheet_pyside()
load_stylsheet_pyside2()
load_stylesheet(pyside=False)
load_stylsheet_from_environment(pyqtgraph=False)

Version 2.8 - next release:

# Still there, remove deprecation message
load_stylsheet_pyqt4()
load_stylsheet_pyqt5()
load_stylsheet_pyside()
load_stylsheet_pyside2()

# Add new API
load_stylesheet(qt_api='') 

# Deprecation warning: Prefer load_stylesheet('pyside2')
load_stylesheet(pyside=True) 

# Deprecation warning: Prefer load_stylesheet(os.environ['PYQTGRAPH_QT_LIB'])
load_stylsheet_from_environment(pyqtgraph=True)

They still using old and new API until the release of v3. I've already pushed this in branch develop.

Version 2.9, 2.10 ....

Keep the same as previous, working on other things if necessary.

Version 3.0:

# Still there, no deprecation message
load_stylsheet_pyqt4()
load_stylsheet_pyqt5()
load_stylsheet_pyside()
load_stylsheet_pyside2()

# Add new API
load_stylesheet(qt_api='') 

# Deprecated: Prefer load_stylesheet('pyside2')
load_stylesheet(pyside=True) 

# Deprecated: Prefer load_stylesheet(os.environ['PYQTGRAPH_QT_LIB'])
load_stylsheet_from_environment(pyqtgraph=True)

@goanpeca agreed?

[matthew-reynolds]

Looks great to me 🎉 Thanks for batting this back and forth with me

[dpizetta]

Implemented the functionality in another way to develop branch, as soon as possible it will be released.