add support for PySide2, #5971 #5972

Merged
merged 2 commits into from Feb 15, 2016

Conversation

Projects
None yet
5 participants
Contributor

thliebig commented Feb 7, 2016

Signed-off-by: Thorsten Liebig Thorsten.Liebig@gmx.de

@thliebig thliebig add support for PySide2, #5971
Signed-off-by: Thorsten Liebig <Thorsten.Liebig@gmx.de>
6b5ea5b

mdboom added the needs_review label Feb 7, 2016

tacaswell added this to the 2.1 (next point release) milestone Feb 7, 2016

Owner

tacaswell commented Feb 7, 2016

Are things more-or-less 'just working'?

Owner

tacaswell commented Feb 7, 2016

the IPython folks would probably appreciate a PR into their shim layer as well.

Contributor

thliebig commented Feb 7, 2016

Since the qt5agg is in place for PyQt5 and PySide2 does have the same interface, so far it is just working for me. Of course there might be problems with PySide2 since it is still in active development, but this should all not concern matplotlib.
We only need to make sure it is possible to request the PySide2 backend and that nothing goes wrong if it is just not installed.

Owner

tacaswell commented Feb 7, 2016

Can you clean up the style issues?

======================================================================
FAIL: matplotlib.tests.test_coding_standards.test_pep8_conformance_installed_files
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/matplotlib/matplotlib/venv/lib/python3.5/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/tests/test_coding_standards.py", line 249, in test_pep8_conformance_installed_files
    expected_bad_files=expected_bad_files)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/tests/test_coding_standards.py", line 143, in assert_pep8_conformance
    assert_equal(result.total_errors, 0, msg)
AssertionError: 3 != 0 : Found code syntax errors (and warnings):
/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/qt_compat.py:68:67: E713 test for membership should be 'not in'
/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/qt_compat.py:68:80: E501 line too long (96 > 79 characters)
/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/qt_compat.py:174:80: E501 line too long (86 > 79 characters)
----------------------------------------------------------------------

I know it is annoying, but consistently enforcing pep8 really does help maintaining a code base the size of mpl with the number of people we have touching the code.

Contributor

thliebig commented Feb 7, 2016

No problem and I can totally understand the idea. What would be the proper way to do it? Add a commit that fixes it or change/amend the old one??
Edit: I just saw that is says to add more commits... ;)

@thliebig thliebig PySide2: fix some problems in qt_compat, #5971
Signed-off-by: Thorsten Liebig <Thorsten.Liebig@gmx.de>
118ef3b
Owner

tacaswell commented Feb 7, 2016

attn @mfitzp

Member

WeatherGod commented Feb 8, 2016

We are flexible. We only request rebase/squashing when a PR has many
commits, particularly with images. Whichever way you'd like to do it is
fine with us.

On Sun, Feb 7, 2016 at 6:34 PM, Thomas A Caswell notifications@github.com
wrote:

attn @mfitzp https://github.com/mfitzp


Reply to this email directly or view it on GitHub
#5972 (comment)
.

Member

mfitzp commented Feb 8, 2016

This looks absolutely fine to me and as @thliebig says the interfaces for PyQt5/PySide2 are otherwise identical (thankfully). I've not got PySide2 built on my system to test it but I can have a go later.

Owner

tacaswell commented Feb 15, 2016

I confirmed that this does not break the existing pyqt5 functionality.

@tacaswell tacaswell added a commit that referenced this pull request Feb 15, 2016

@tacaswell tacaswell Merge pull request #5972 from thliebig/master
add support for PySide2, 

closes #5971
f8d7114

@tacaswell tacaswell merged commit f8d7114 into matplotlib:master Feb 15, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

tacaswell removed the needs_review label Feb 15, 2016

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