search

jmitrevs / matplotlib_backend_qtquick

A QtQuick backend for matplotlib
Other
20 stars 8 forks source link

Remove the `qt_compat` module. #7

Open appetrosyan opened 3 years ago

appetrosyan commented 3 years ago

Hi. This is just a suggestion, but I have found that providing the compatibility as seen by matplotlib_backend_qtquick.qt_compat module is more trouble than its worth. I understand that this is because you don't want to maintain two codebases one that provides a PyQt5-compatible canvas, and a PySide2 compatible one. I also understand that you don't want people to strip out the part that does the heavy lifting and not use the rest of your package, and that's really the alternative here.

It is common for people to have both PyQt5 and Pyside installed at the same time. If PyQt5 got installed as a dependency for one package, and the developer (as is common) developed a PySide2 application with only PySide2 installed, they won't have trouble related to the way in which matplotlib_backend_qtquick chooses the backend. As a result, the compatible version of QtCore can be from PyQt5, while the rest of the application is written in PySide2. This breaks packages.

The way in which it chooses is .... not great. If I'm using your package it's likely that I assume I should import only your versions of QtCore and so on. If I only have PySide2 installed I assume that your package is written in PySide2 compatible python. I then deploy it to the client's machine and lo and behold the users get a cryptic traceback about incompatible types.

Rather than try and make the clever bit more clever, I suggest that the compatibility mode be made explicit. In lieu of importing matplotlib_backend_qtquick.qt_compat I suggest a different structure: matplotlib_backend_qtquick.generic the one that guesses from the context, matplotlib_backend_qtquick.PySide2 for the PySide2 compatible versions, and matplotlib_backend_qtquick.PyQt5 for the PyQt5 respectively.

This will save developers lots of headache and be the deciding factor between using your package, or writing one based off of your source code.

congzhangzh commented 3 years ago

I agree, it will be better to directly support the modern qt for python api.

jmitrevs commented 3 years ago

This part was largely copied from matplotlib, as I mention in the docstring, with minor updates so that QtQuick and QtQml were also imported.

https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/backends/qt_compat.py

Probably there is a better way to avoid the code duplication. It was a bit of a rushed addition. Originally I tested the backend only with PySide2. How does matplotlib avoid the issue you are referring to? I didn't want to deviate much in this case, but I could have introduced a bug in my version.

appetrosyan commented 3 years ago

How does matplotlib avoid the issue you are referring to? I didn't want to deviate much in this case, but I could have introduced a bug in my version.

it doesn’t. There are two code paths throughout Matplotlib, unnecessarily so: the only reason you’d choose PyQt5 over Pyside2 is the license, and MPL is GPL.

This part was largely copied from matplotlib

To be quite honest, copying any architectural decision from matplotlib is a bad idea. People use it despite its shortcomings, because of mindshare and wishing not to fragment the Python user base. It has a lot of API baggage, and a lot of ... shall we say... things that are too late to fix, but are undoubtedly bad decisions, and being compatible with both, and not having the two code paths separated into a Pyside2 module and a PyQt5 module is one of those.

Anyway.

appetrosyan commented 3 years ago

Would you accept a PR that provided two separate modules, a pyside2_compat and a pyqt5_compat? I would not remove the current version yet, but add a deprecation warning and tell the users of the package to explicitly specify the Qt bindings library? I’ll also make some modifications to the examples to reflect this.

congzhangzh commented 3 years ago

I think it's very hard to keep comptiable in the future, as Qt is keep changing, and so for qt for python.

As matplotlib is more api stable(marebe), A better choice maybe split the qt backend for matplotlib and keep sync with qt upstream?

I have a demo project which lack many function currently, but maybe a good start point to easy community to improve?

The idea project:

https://github.com/medlab/matplotlibqml