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

PR: Add a warning for developers still running legacy Qt4-based APIs #283

Merged
merged 1 commit into from
Nov 18, 2021

Conversation

CAM-Gerlach
Copy link
Member

@CAM-Gerlach CAM-Gerlach commented Nov 17, 2021

We will now warn developers (via a DeprecationWarning, so they have to be running Python with -X dev, -W default or -W error, or import QtPy in their __main__ script) when they are using long-deprecated, soon to be removed Qt4 based APIs (PySide and PyQt4), either via their explictly set preference, via import, or via a default fallback if installed, all of which will be unsupported come QtPy 2.0.

Resolves #261

@CAM-Gerlach CAM-Gerlach added this to the v1.11.3 milestone Nov 17, 2021
@CAM-Gerlach CAM-Gerlach self-assigned this Nov 17, 2021
@CAM-Gerlach
Copy link
Member Author

Looks like the CIs are not running at all. I'll see if I can fix that...

@CAM-Gerlach
Copy link
Member Author

The issue is the old CI job, before my improvements, only runs on master. I'll modify it here so it triggers as well (so long as it works being in a fork from my repo, which is the thing I had trouble with before, otherwise I'll have to do it in another branch/PR here).

@CAM-Gerlach
Copy link
Member Author

(So, incidentally, PR #281 skipped the tests and checks completely)

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @CAM-Gerlach!

@CAM-Gerlach
Copy link
Member Author

I amended it into the same commit, to hopefully make it a little easier when merging to master (since adding the warning and the release branch pattern will need to be dropped when merging; LEGACY_APIS can stay.

@CAM-Gerlach
Copy link
Member Author

@ccordoba12 Want me to merge when the CIs pass, or should we wait for @dalthviz ?

@dalthviz
Copy link
Member

Is there any need to merge 1.x with master after this PR @CAM-Gerlach ? In the merge we will need to readd PyQT4_API and PYSIDE_API or change LEGACY_APIS to be ['pyqt', 'pyqt4', 'pyside'] while the rest of the changes are dropped, right?

@ccordoba12
Copy link
Member

@dalthviz, you can still merge with master and drop all changes present here (because they don't apply) by doing an empty commit for the merge.

dalthviz added a commit that referenced this pull request Nov 18, 2021
@CAM-Gerlach
Copy link
Member Author

Ah, I somehow forgot that LEGACY_APIS also required having those other constants, which probably aren't harmful but realistically aren't really needed, so I guess a just doing a null merge is fine (to still keep the branches in sync of course). Thanks for taking care of it, @dalthviz .

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

Successfully merging this pull request may close these issues.

Add a deprecation warning for unsupported Qt versions and bindings (at least Qt4: PyQt4 and PySide)
3 participants