🚧 Add basic qtpy layer

[GuillaumeFavelier]

I think this PR could help #8 by using a thin qt abstraction layer.

The goal would be to have one code and change the QT_API accordingly or possibly auto-detect it.
Basic testing with PySide2 would be nice to have too.

[GuillaumeFavelier]

What is missing now is synchronizing qtpy and vtk with the same implementation I think

[GuillaumeFavelier]

When I use PySide2 as Qt backend, something wrong happens during the transfer of the parameters from QVTKRenderWindowAdapter to BasePlotter, the parameters dedicated to BasePlotter are evaluated as Qt signal or property (example with shape):

AttributeError: 'shape()' is not a Qt property or a signal  

I evaluated the mro of each of the backends:

with PySide2:

[
	<class 'pyvistaqt.plotting.BackgroundPlotter'>,
	<class 'pyvistaqt.plotting.QtInteractor'>,
	<class 'pyvistaqt.plotting.QVTKRenderWindowInteractorAdapter'>,
	<class 'PySide2.QtCore.QObject'>,
	<class 'Shiboken.Object'>,
	<class 'pyvista.plotting.plotting.BasePlotter'>,
	<class 'pyvista.plotting.picking.PickingHelper'>,
	<class 'pyvista.plotting.widgets.WidgetHelper'>,
	<class 'object'>
]

with PyQt5:

[
	<class 'pyvistaqt.plotting.BackgroundPlotter'>,
	<class 'pyvistaqt.plotting.QtInteractor'>,
	<class 'pyvistaqt.plotting.QVTKRenderWindowInteractorAdapter'>,
	<class 'PyQt5.QtCore.QObject'>,
	<class 'sip.wrapper'>,
	<class 'sip.simplewrapper'>,
	<class 'pyvista.plotting.plotting.BasePlotter'>,
	<class 'pyvista.plotting.picking.PickingHelper'>,
	<class 'pyvista.plotting.widgets.WidgetHelper'>,
	<class 'object'>
]

So I can only assume that there is something different in the way the parameters are handled somewhere in PySide2 compared to PyQt5 (or maybe PyQt5 does something actually):

	<class 'PySide2.QtCore.QObject'>,
	<class 'Shiboken.Object'>,

At this step, I don't really want to mess with the inheritance sequence considering all the constraints:

• inherit QObject
• inherit QtInteractor
• avoid duplicated call to BasePlotter.__init__()
• allow multiple inheritance (QVTKRenderWindowInteractor, BasePlotter)

Maybe someone else can have a look at the mro and notice something strange that I missed?

Meanwhile, I'll keep thinking about the architecture.

[GuillaumeFavelier]

I don't see any solution here without changing the architecture.

[GuillaumeFavelier]

The architecture has been changed very recently in #25.

Let's give it a try again.

[GuillaumeFavelier]

It works for me locally with PySide2 but my ipython hangs after closing the MainWindow.

[GuillaumeFavelier]

And I downgraded PySide2 to 5.14.2, I don't have this issue at all.

[codecov]

Codecov Report

Merging #12 into master will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #12   +/-   ##
=======================================
  Coverage   94.90%   94.90%           
=======================================
  Files           5        5           
  Lines         432      432           
=======================================
  Hits          410      410           
  Misses         22       22           

[akaszynski]

Code coverage is working!

[GuillaumeFavelier]

Code coverage is working!

The coverage diff yes :), but the badge on the project is not updated:

[larsoner]

@GuillaumeFavelier is this good to go? Maybe a rebase now that master has coverage working?

[GuillaumeFavelier]

is this good to go?

This basic version should work but I would like to improve it further. For example in the case where both PySide and PyQt5 are installed, I need to make sure that QtPy and VTK use the same python Qt binding. Plus I want to rework the testing for it.

Maybe a rebase now

I'll rebase asap 👍

[nicobako]

Hey. Just wondering what's holding this issue up? I actually ran the code locally on my machine and it seems to be working. My use case is that I really want to use pyvistaqt, but the pyqt5 licence is stopping me. Using qtpy instead of pyqt5 is a genius move that would allow users more freedom. I want to use pyside2, and it seems to be working.

Do you need help to get this done? Maybe update the documentation? The code examples would definitely may need to be updated. I can help.

[nicobako]

@larsoner , I see that @GuillaumeFavelier has not been active on this pull request for quite some time. I hope I'm not stepping on any toes, but if it would be helpful I can help with this issue.

I fetched Guillaume's branch and updated it with the pyvista master branch, and made some other small changes to get things working. I also updated the test files. I have submitted a pull request GuillaumeFavelier#1 to Guillaume's branch in the hopes that, if he accepts the changes, they will be automatically incorporated into this issue. If it's easier I can create a new pull request. Just let me know.

Anyway, I hope I'm not offending anyone. I just want to be helpful. Plus, I have some time now to work on this issue. Being able to use pyvistaqt with qtpy instead of pyqt5 would be a really wonderful thing, and would make pyvistaqt more usable to people who are afraid of pyqt5's license.

[larsoner]

but if it would be helpful I can help with this issue.

Agreed we should move forward with this, I doubt @GuillaumeFavelier will mind. Feel free to start your own PR with his commits cherry-picked, then yours on top (with a merge or rebase with latest master to deal with the merge conflict). Or just start fresh without his commits if it's easier. @GuillaumeFavelier is a maintainer so as long as you check the "allow edits by maintainers" box @GuillaumeFavelier can automatically push commits to your branch, too, to continue working on it.

[nicobako]

Thank you. My work was a continuation of his. I fetched his branch and just added commits to it. I'll create a new PR really soon.

[GuillaumeFavelier]

@nicobako you're right, I didn't work on this for a while, thanks for the push! :)

[GuillaumeFavelier]

Closing as duplicate of #61

[adam-grant-hendry]

I know I am coming to the party late, but I wanted to comment that the approach used here is proper on several levels. Not only does matplotlib also do this in FigureCanvasQT, but it is the recommended approach by Raymond Hettinger in his PyCon 2015 talk Super considered super! and his corresponding blog post.

Please see my comment here on matplotlib's repo. I believe the fact that PyQt and PySide only accept keyword arguments for optional arguments coupled with PySide's lack of support for cooperative multiple inheritance are the root cause of the issue.