Fix qt backend on mac big sur #19334
Conversation
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
|
Seems reasonable. @dstansby you mentioned having Big Sur? (ahem, I suppose I should upgrade one of my machines one of these days, but...) |
|
On one hand, this seem to have only moderate down side, but if it is fixed in newer versions of (py?)Qt it seems to be a moot point? |
|
I'm struggling to reproduce the original bug - I think it's worth discussing at #18954 (comment) before going further with this. |
|
I can reproduce the original issue, and confirm that setting this environment variable fixes it. |
|
I was wondering if this would break older versions of OSX/macos, but #18134 says QT5 requires > 10.13, and https://bugreports.qt.io/browse/QTBUG-87014 seems to say that setting this should be fine going back to 10.13, so this looks good to me. |
|
Does anyone have thoughts on backporting this to 3.3.x? Seems reasonable to me. |
76e8260 adds an additional condition |
| # Fixes issues with Big Sur | ||
| # https://bugreports.qt.io/browse/QTBUG-87014, fixed in qt 5.15.2 | ||
| if (sys.platform == 'darwin' and | ||
| LooseVersion(platform.mac_ver()[0]) >= LooseVersion("10.16") and |
There was a problem hiding this comment.
I don't know if this should be a >= or == and I don't know if we can even know how is it going to be with the next MacOS release...
|
The choices are:
Qt/pyqt is generally pretty good about back-compatibility / not breaking users so if we are going to do this, the version filtering seems right. My main hang up about this is still that libraries should probably not be setting envs (as that is users / application concerns). |
I agree that we shouldn't be doing this but in practise this is the best and less intrusive solution I know or I can think of. If the |
|
This is compelling argument. Thanks @ericpre ! The UI is a bit unclear, but it looks like this is your first PR to Matplotlib 🎉 . Congratulations and hopefully we hear from you again! |
|
Thanks everyone for the review! |
PR Summary
The qt backend is not working on mac big sur as reported in #18954.
It is fixed in qt 5.15.2 (https://codereview.qt-project.org/c/qt/qtbase/+/322228) and in the coming 5.12.11 (https://codereview.qt-project.org/c/qt/qtbase/+/322507) but setting this environment variable is necessary for qt <5.15.2.
Even if in a ideal word
matplotlibshouldn't be setting this variable environment, this is pragmatic and very convenient fix!PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).