ENH: improve PySide2 loading#7252
Conversation
Don't require explicit QT_API setting for PySide2 in qt_compat.py
tacaswell
changed the title
| if 'PySide2' in sys.modules: | ||
| # user has imported PySide before importing mpl | ||
| QT_API = QT_API_PYSIDE2 | ||
|
|
There was a problem hiding this comment.
Why don't we use if elif checks instead of consecutive if statements?
There was a problem hiding this comment.
These things are going to be mutually exclusive due to restrictions from the libraries.
This could be cleaned up, but I think is out of scope of this PR.
|
There is also a pep8 failure. |
lib/matplotlib/backends/qt_compat.py
Outdated
| raise RuntimeError( | ||
| ('Unrecognized environment variable %r, valid values are:' | ||
| ' %r, %r or %r' % (QT_API_ENV, 'pyqt', 'pyside', 'pyqt5'))) | ||
| ' %r, %r or %r' % (QT_API_ENV, 'pyqt', 'pyside', 'pyqt5', 'pyside2'))) |
There was a problem hiding this comment.
this line needs to be wrapped.
There was a problem hiding this comment.
I also didn't adjust the number of argument placeholders. I fixed both issues.
NelleV
changed the title
|
Apart from the pep8 comment, this looks good. |
Fixed number of arguments and pep 8 issue
|
There is still a pep8 issue: there is a trailing whitespace l:72 You can easily test pep8 compliance by installing the pep8 tool and configure your text editor to show this to you inline. (We are currently working on our contribution guidelines to reflect this). |
removed trailing white space
|
thanks for the feedback. installed pep8 and should be fixed now. |
|
Thanks for putting up with our style checking. It really does help keep the code base consistent |
|
The failures are all unrelated. |
|
@jrversteegh Thanks! |
QuLogic
changed the title
6 participants
Don't require explicit QT_API setting for PySide2 in qt_compat.py