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

Use exec instead of exec_ for Qt dialogs #795

Merged
merged 1 commit into from
May 14, 2024

Conversation

naglis
Copy link
Contributor

@naglis naglis commented May 1, 2024

exec_() is being deprecated in favor of exec().

Also use launch() helper method for Dialog subclasses.

Note: there is one case of exec_() left, however I've found that under PySide2 QApplication does not have an exec() method. Also, this use of exec_ does not seem to produce warnings.

Fixes #595

@deeplow
Copy link
Contributor

deeplow commented May 7, 2024

Thank for the PR and for fixing something which we've been neglecting for quite a while (since we added PySide6 support).

Note: there is one case of exec_() left, however I've found that under PySide2 QApplication does not have an exec() method. Also, this use of exec_ does not seem to produce warnings.

This fact was bothering me, so I dug a little deeper. PySide2 supported python2, which means that there it could not use exec since it was a reserved keyword (at least this plausible explanation for PyQt5 makes sense here as well).

This also explains why exec_() is not found on the C++ Qt5 docs (the PySide2 equivalent). On top of that, on the PySide2 docs, even though exec_() for QApplication has a docs entry, while exec() does not, the examples (which come from translated C++, I guess) use exec.

In conclusion, since Python2 has now reached its end of life, I think we can assume that exec() will be the de-facto way forward and that QApplication.exec_() working on PySide6 was just some developer forgetting to deprecate it. However, monkeypatching it after the PySide2 import is a bit of a mypy can of worms, so I guess we'll just have to leave it as-is for now 😞.

In any case, if QApplication.exec_() ever stops working, we notice it, right away, so it's not a big deal.

@naglis
Copy link
Contributor Author

naglis commented May 7, 2024

This fact was bothering me, so I dug a little deeper. PySide2 supported python2, which means that there it could not use exec since it was a reserved keyword (at least this plausible explanation for PyQt5 makes sense here as well).

This also explains why exec_() is not found on the C++ Qt5 docs (the PySide2 equivalent). On top of that, on the PySide2 docs, even though exec_() for QApplication has a docs entry, while exec() does not, the examples (which come from translated C++, I guess) use exec.

Thank you for the extra analysis @deeplow! I was a bit baffled by exec() being mentioned in the Qt5 docs but the method not existing, now it is more clear why that might be the case.

@apyrgio
Copy link
Contributor

apyrgio commented May 8, 2024

Thanks a lot for the PR @naglis. Same as in #794, since we don't need to rush this PR, I'll merge it after the 0.6.1 release.

`exec_` is being deprecated in favor of `exec`.

Also use `launch()` helper method for `Dialog` subclasses.

Fixes freedomofpress#595
@apyrgio apyrgio merged commit 8694fb2 into freedomofpress:main May 14, 2024
6 of 11 checks passed
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.

PySide DeprecationWarning: exec_->exec
3 participants