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

Warn if using a deprecated and/or unsupported Qt 5 version #284

Closed
CAM-Gerlach opened this issue Nov 19, 2021 · 12 comments
Closed

Warn if using a deprecated and/or unsupported Qt 5 version #284

CAM-Gerlach opened this issue Nov 19, 2021 · 12 comments
Assignees
Milestone

Comments

@CAM-Gerlach
Copy link
Member

Since #261 was implemented in PR #283 , we're now issuing a DeprecationWarning for QtPy 1.x when used with Qt versions and bindings that are deprecated, untested in QtPy 1.x, and unsupported in QtPy 2.0.

Given Qt/PyQt versions <5.9 and Pyside2 versions <5.12 are equally deprecated, untested in QtPy 1.x and officially unsupported in QtPy 2.0 (since we've removed any compat code for them, AFAIK), it would seem to make sense to issue an equivalent DeprecationWarning in QtPy 1.x for that case as well.

Furthermore, we might want to elevate the warning to a UserWarning or a PythonQtError in QtPy 2.0, given that serious runtime breakage may occur, just as we currently do in 2.0 for the Qt 4 bindings, so users see it up front rather than having things mysteriously break later.

Depending on what is decided in #233 or elsewhere for future QtPy 2.0 versions going forward, we could also consider introducing a DeprecationWarning in QtPy 2.0 for PyQt <5.12 in that version, if in #233 it is decided to that be our support target for future follow-on versions. However, if that's not immediately decided or out of scope here, that could be punted again to another followup issue.

Relevant conversation history on #261:

I said

We might want to consider a warning if a version of PyQt (e..g. < 5.9 or < 5.12), and/or PySide2 (e.g. < 5.12) is used that would similarly be unsupported in QtPy 2.0.0, and untested in recent QtPy 1.x.

@dalthviz said

Thats true, yes, I would say that the acceptable minimum for PyQt could be >=5.9 (since is the lates version in the anaconda channel) but since there are some issues regarding scoped enums with that version for Python 3.6 and 3.7 probably at the end the minimum will be something like >=5.12 as with PySide2. However we need to further discuss this too

I said:

I assumed this issue was motivated by the fact that Qt4 binding support is removed in QtPy 2.0, untested and deprecated in QtPy 1.x, and EoL upstream. Since all three of these reasons are equally true for Qt5 <5.9/5.12 (the specifics of which are being discussed in #233 ), I thought it might make sense to consider warning for that here as well (considering I'd think we'd want to decide on that before you make a final QtPy 1.x release with this change).

@dalthviz said

Ah yes that makes sense, will reword this issue then 👍

I said

@ccordoba12 Oh, so we already warn on Qt <5.6 and/or <5.9?

@dalthviz said

Also @ccordoba12 you think that in the message we should add something regarding Qt5 ? (Like a minimum version)

I said

point_up Since Qt bindings < 5.9-5.12 (per the discussion on #233 ) are also equally deprecated and no longer supported in QtPy 2.0, my suggestion was that we should also include those in this warning too.

@ccordoba12 said

I don't think so because qtpy takes care of that for the user.

I said

@ccordoba12 Oh, so we already warn on Qt <5.6 and/or <5.9?

@ccordoba12 said

No, we don't. Perhaps we should warn about Qt < 5.9 because the oldest version we can test.

@dalthviz said

Checking again the comments, regarding the message for Qt 5 <5.9, that's going to be done by @CAM-Gerlach in a new PR, right?

@ccordoba12 said

I think we should leave this message for QtPy 2.0

I said

I'm not sure about that, but we can discuss that in the new issue.

Originally posted by @CAM-Gerlach in #261 (comment)

@CAM-Gerlach CAM-Gerlach self-assigned this Nov 19, 2021
@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Nov 19, 2021

@ccordoba12 said in response to @dalthviz on #261 :

I think we should leave this message for QtPy 2.0

The issue is that PyQt <5.9/Pyside <5.12 is already likely broken in 2.0, since it hasn't been tested for years and any explicit compat code was removed, so developers will not be warned about it. For the reasons explained above, the very same ones that justified a DeprecationWarning for Qt4, I think we should have one for those bindings in 1.13.3 so developers are made aware, and then in 2.0 we upgrade that to a UserWarning or (like for other unsupported bindings) a PythonQtError, since things are likely to break at runtime.

However, particularly since it looks like we'll still have it for the initial 2.0 release, the DeprecationWarning for dropping Qt and PyQt <5.12 support should go in 2.0 only, and then can be upgraded to a UserWarning or PythonQtError in a future (major, if an error) release.

@dalthviz
Copy link
Member

I think we should show an UserWarning or maybe a RuntimeWarning on 2.0 if we detect that QtPy is being used with Qt <5.12

Maybe the message can go along the lines of saying something like: We detected that you are using an old version of Qt 5. We recommend you to update to a more recent version (Qt >5.12)

Regarding that detection on 1.11.3, I'm not totally sure what type of warning or message to put.

@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Nov 22, 2021

I think we should show an UserWarning or maybe a RuntimeWarning on 2.0 if we detect that QtPy is being used with Qt <5.12

Under PyQt 5.9 is what is untested and deprecated, so I wouldn't suggest a user-visible UserWarning on PyQt >=5.9 at least until a newer version is available on Anaconda defaults (or its clear there will never be one), since we do test it, and developers and users have no choice there and it means that it will issue a warning in Spyder itself. However, if we have confirmed plans to deprecate it, we could certainly start issuing a DeprecationWarning with QtPy 2.0+.

Regarding that detection on 1.11.3, I'm not totally sure what type of warning or message to put.

The rationale is identical to the DeprecationWarning for Qt4, so if we don't have a strong reason or empirical experience to suggest those versions are significantly more or less broken then Qt4, or there's something else to justify it, it would make sense to be consistent and issue the same warning. However, if you think that its more likely that it is the application that is requiring the Qt4 bindings, but more likely the environment that controls the Qt point versions, then I could it see it making sense to warn users instead, though it does seem a bit odd to issue a much less visible warning on an older version and a more prominent one on a newer one.

@dalthviz
Copy link
Member

So a DeprecationWarning for both QtPy 1.11.3 and 2.0.0 versions regarding Qt < 5.9 looks ok for you, right @CAM-Gerlach ?

Also, just in case, what do you think @ccordoba12 ?

@CAM-Gerlach
Copy link
Member Author

Yup, that's my take, with the following revisions as mentioned above:

  • PyQt 5.9 and PySide 5.12 (since it wasn't stable/supported until then/QtfP was released, IIRC)
  • I might suggest a UserWarning/etc. instead for QtPy 2.0 instead, since the code is likely to be actively broken, its harder to miss than a DeprecationWarning and individual Qt minor versions are more likely than major to be controlled by the user's environment rather than an application developer

So I'd suggest

A DeprecationWarning for QtPy 1.11.3 and a UserWarning/subclass with 2.0.0 for PyQt <5.9 and PySide <5.12

In addition, if and when we confirm plans to deprecate and eventually remove PyQt <5.12 on QtPy 2.x, we would want to add a DeprecationWarning for that on QtPy 2.x only.

@dalthviz
Copy link
Member

Makes sense to me 👍

@dalthviz
Copy link
Member

Do you have some time to work on this @CAM-Gerlach ? I think that after this is done we could release 1.11.3

@dalthviz dalthviz modified the milestones: v2.0.0, v1.11.3 Nov 29, 2021
@CAM-Gerlach
Copy link
Member Author

Sure; I was working on python/peps#2164 (PEP 639 rewrite), sorry, which I just finally finished. I'll take care of it right now.

@ccordoba12
Copy link
Member

So a DeprecationWarning for both QtPy 1.11.3 and 2.0.0 versions regarding Qt < 5.9 looks ok for you
Also, just in case, what do you think @ccordoba12 ?

Although it seems you already decided how to proceed, I just want to say that I also agree with this.

@CAM-Gerlach
Copy link
Member Author

@ccordoba12 FYI, we revised that slightly since that comment; here is the current consensus (as is presently implemented in PR #289 )

A DeprecationWarning for QtPy 1.11.3 and a UserWarning/subclass with 2.0.0 for PyQt <5.9 and PySide <5.12

In addition, if and when we confirm plans to deprecate and eventually remove PyQt <5.12 on QtPy 2.x, we would want to add a DeprecationWarning for that on QtPy 2.x only [but that's out of scope here].

@ccordoba12
Copy link
Member

we would want to add a DeprecationWarning for that on QtPy 2.x only

Yes, I think that's a good idea too.

@CAM-Gerlach
Copy link
Member Author

Yes, I think that's a good idea too.

Great, happy to help with that too; just ping me if/when that is decided and for what version.

dalthviz added a commit that referenced this issue Dec 1, 2021
PR: Add warning for deprecated/EoL Qt5 & PyQt5/PySide2 versions to fix #284
@kumattau kumattau mentioned this issue Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants