8000 Qt5 Backend by badders · Pull Request #2471 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Qt5 Backend #2471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Qt5 Backend #2471

wants to merge 3 commits into from

Conversation

badders
Copy link
@badders badders commented Sep 27, 2013

I have created a qt5 backend based on the existing code for the Qt4 backend. Everything i've tested seems to be working so far. Unortunately there are lots of namespace changes between pyqt4 and pyqt5, and some other small subtle differences that prevent the two backends being identical. Due to some differences with how pyqt5 handles multiple inheritence, this backend will only work with python3 and above, as pyqt5 multiple inheritence requires use of the new super() function.

@mdboom
Copy link
Member
mdboom commented Sep 27, 2013

Thanks for working on this.

Can you be specific about what some of the namespace changes really are? I'm really wary of adding yet another backend through copy-and-paste if we can avoid it. The current qt4 backend supports with PyQt4 and PySide through a namespace abstraction in qt4_compat.py. I wonder if it's possible to instead extend that. I think it will be illustrative to diff backend_qt5.py and the existing backend_qt4.py, which I haven't had the chance to do...

Note that Python 2.6 and later have the super function. It's more automatic in Python 3, but it is at least available on Python 2, though more verbose. Can you be more specific about why the Python 2 super is insufficient?

@badders
Copy link
Author
badders commented Sep 27, 2013

Basically lots of stuff moved from QtGui to QtWidgets, some things now have a pyqt prefix. Python 2 super might be sufficent, but as far as i can tell pyqt5 only works on python 3 and above anyway, at least i cant get it to work on python 2.7 (it also actually has support for building against qt4 and just dropping things that dont exist for added confusion). I did look at using the compatibility early on, but couldnt find a way of making that work.

My super comment is going from the docs on the pyqt5 website, they basically dont support multiple inheritence at all but say it will work with python 3s automatic super, which does seem to have different behaviour from python 2 super anyway. I also suspect the Qt4 backend actually wont work on python3 due to inheritence differences, but without being able to test this i could be wrong. You can diff the relevent qt4 and qt5 equivalent files and see what changes i've had to make. Unfortunately i've fixed all the pep8 errors which might add some noise to that diff.

@badders
Copy link
Author
badders commented Sep 27, 2013

One difference ive noticed with inheritence between python 2/3 is when you directly call Parent.init_, in python 2 it just uses that super class, but in python 3 it seems to call alll the super class init methods, even though you only explicitly call one. I didnt want to add a copy/paste backend for obvious maintenance reasons, but i suspect they will quickly diverge as things like retina support on osx (which i believe is much improved in qt 5.1), and QML2 and QtQuick support are added

@tacaswell
Copy link
Member

I think we want to keep supporting pyside, which I assume will pick up qt5 support eventually (I seem to think it will 'just work', but not sure why I think that) anything that has 'pyqt' in it's name should be laundered through a compatibility layer.

@badders
Copy link
Author
badders commented Sep 28, 2013

Since the whole nokia mess pyside has been much more inactive, and no longer offers anything over the officially supported pyqt

@badders
Copy link
Author
badders commented Sep 28, 2013

And pyside support can be merged later anyway if qt5 support improves, that shouldnt impact on a working backend now.

@tacaswell
Copy link
Member

pyside has licensing benefits.

@mdboom
Copy link
Member
mdboom commented Sep 30, 2013

I'll second @tacaswell's comment, that PyQt continues to be GPLv3 which is a problem for some. pyside is far from dead if the amount of effort going into it at SciPy2013 is any indication, with official support coming from (at least) Enthought and Valve.

@mfitzp
Copy link
Member
mfitzp commented Dec 19, 2013

I'd like to see this backend available to use Matplotlib in a PyQt5-based application (unfortunately Qt4 isn't an option). Is this pull-request in a usable state @badders? When you say "this backend will only work with python3 and above" does that mean it's impossible for Matplotlib to work with PyQt5 on Python 2.7 or would the wrapper need to be tweaked to use old-style inheritance?

Happy to help in any way to move this along. Thanks

@badders
Copy link
Author
badders commented Jan 13, 2014

It should still work, i havent had any time at all to work on it recently, been too busy with real work. I dont think its impossible to work with python 2.7, but i could only get pyqt5 to work with python 3, so its untested on python 2. Its possible there may need to be some changes as i had to modify some of the constructors to work in python 3.

@hadim
Copy link
hadim commented Feb 27, 2014

Up here ?

@mfitzp
Copy link
Member
mfitzp commented Feb 27, 2014

To update on my earlier comment - I've got this working on Python 2.7 (see the files here: https://github.com/pathomx/pathomx/tree/master/pathomx ) with PyQt5. It was relatively straightforward to do - I'd be happy to construct a pull-request for it.

There were a few additional Qt5 things that needed doing - particularly changing the mouse event handling which (at least in PyQt5) has changed considerably.

@mfitzp
Copy link
Member
mfitzp commented Feb 27, 2014

@mdboom On the subject of namespace changes from Qt4 to Qt5 you can see an example of a wrapper to "undo" these here https://github.com/mfitzp/pyqtgraph/blob/pyqt5/pyqtgraph/Qt.py (from line 110) basically by reassigning the objects back under the Qt4 namespace and re-implementing deprecated APIs (using the example code from Qt re-written into Python). I'm not suggesting we do that here, but it gives an indication of the scale of the problem.

As mentioned above there are also issues in changes to mouse wheel event handling due to the Qt5 support for high-resolution trackpads (pixelDelta vs. angleDelta). This is probably easily circumvented by sub-classing though.

@mdboom
Copy link
Member
mdboom commented Feb 27, 2014

@mfitzp: If you're interested in working up a PR with that alternate approach (of adapting the namespace, rather than copy-and-pasting the backend whole cloth), that would be awesome. I think it's also fine if the qt5 backend inherits from and overrides a few key things in qt4 if necessary to address the mouse event behavior as you say.

@erikhvatum
< 8000 /span> Copy link

@mdboom If/when pyside or pyotherside supports Qt5, I would expect it to expose the QtGui and QtWidgets namespaces and Qt5 mouse events as Qt5 presents them. So, a parallel Qt5 code path isn't necessarily some kind of PyQt5 specific thing that leaves pyside in the cold. We could do for Qt5 what we do for Qt4: offer shims for both pyside and pyqt. During the interval where we support both Qt5 and Qt4, there would be some near-duplicate code.

Is the Qt code often modified? If it's pretty much static, there's not much duplicate maintenance to do.

@mdboom
Copy link
Member
mdboom commented Mar 11, 2014

My primary concern is adding another backend through cutting and pasting if there's a cleaner way to avoid it. I understand PySide and PyQt can be handled with a common code base (we already do that for Qt4) -- it would be nice to extend that for Qt5 as well. The Qt code is not edited terribly often, but the cost of each additional backend turns out to be rather high, since for any internal change that requires a change to the backend API we have to make that change N times, once for each backend.

@tomcraw00
Copy link

Is this ever going to be supported? There are discussions going back 6 months with no resolution. A working solution was provided but it seems stuck in philosophical discussion. I would help but unfortunately I'm not able to provide any solution more elegant than making another backend.

@mfitzp
Copy link
Member
mfitzp commented Mar 20, 2014

@tomcraw00 It's not a philosophical issue it's a time issue. I'm quite happy to implement this (I have a working implementation already, derived from the @badders work) but haven't got around to it as I've been moving house.

If you want a quick and dirty solution just use the two files named backend_ from here: https://github.com/pathomx/pathomx/tree/master/pathomx

Next week I'm skiing, but after that?

@tomcraw00
Copy link

@mfitzp thanks, I will give that a try. @mdboom seemed to take exception to adding another backend, PySide support, etc. Just trying to help generate some momentum :) I can certainly help test on my application.

@tacaswell
Copy link
Member

@tomcraw00 Future maintainability also needs to be taken into consideration as do licensing issues. QT4 will be supported for a long time (when do you think the last version of RHEL without QT5 will be EOLd?). We will get the qt5 stuff in eventually, but I don't really see the urgency yet (please correct me if I am wrong).

@mfitzp Be careful about IP stuff, if you have read the gpl'd version in pyqt, you probably can not write a BSD version for mpl (and we most definitely can not copy out parts of it). Riverbank made it quite clear that they are not interested in more permissive licensing and we must respect that.

Using an external backend is quick, but I disagree it it dirty. The ability to easily use third-party backends is an explicit design goal, just because mpl does not want to carry the maintenance burden/gpl only code, does not mean some one else can't. It might even be worth setting up a separate repo for the code in phantomx.

@mfitzp
Copy link
Member
mfitzp commented Mar 20, 2014

@tacaswell I'm sorry don't follow re the "gpl'd version in PyQt"? GPL'd version of what? The qt5 code here is a copy-paste of the existing qt4 backend (taken from mpl) with fixes for the changes in module structure. I don't see the licensing issue.

The linked code is 'quick and dirty' in that it's partially broken. When I said "working implementation" I meant it works for my needs (as badder's did for him - i.e. no mouse event support which I've subsequently added). There are a few more things to fix before it's ready.

I'll see if I can get an acceptable backend for merging into mpl first - but if not a separate repo would be a very good idea. Thanks.

@tacaswell
Copy link
Member

@mfitzp sorry, I miss-read your earlier comment "On the subject of namespace changes from Qt4 to Qt5 you can see an example of a wrapper to "undo" these here..." to be a link to the Riverbank code.

@tomcraw00
Copy link

@tacaswell I'm more concerned with Qt and PyQt support not being official in Mac OS X 10.9. Compiling PyQt 4.10.X produces warnings that the operating system isn't supported. Clearly not an actual issue on 10.9 but with OS X's yearly upgrade cycle and the inability to downgrade an OS I wanted to get my userbase at my company moved over sooner rather than later.

@mdboom
Copy link
Member
mdboom commented Mar 20, 2014

For the record, I don't take exception with adding support for PyQt5 (and we already support PySide). I take exception to doing it through copying-and-pasting of an existing backend. I much prefer to adapt the namespace to be able to support all of them from a single code base.

@tomcraw00
Copy link

@mfitzp for what it's worth your backend files work with my application so far. Basic histograms, box plots, scatter, etc

@badders
Copy link
Author
badders commented Mar 23, 2014

Sorry i haven't had any time to work on this, been far too busy with other things and this is unfortunately not a high priority. Last time i checked, the qt4 backend code will also be broken on python 3, has anyone tested this recently?

What are the licensing issues exactly with using the existing qt4 backend code with the qt5 backend? I understand pyside is under a more permissive license, but beyond that I dont know exactly what the concerns are? Is there a difference between licensing of qt4 and qt5?

@tohojo
Copy link
tohojo commented Mar 23, 2014

I have spent the weekend building a PyQt4 application embedding matplotlib, and it works fine in both python 2 and 3 as far as I can tell (at least for python 3.3 / pyqt 4.10). :)

@liuyu81
Copy link
liuyu81 commented Mar 25, 2014

@WeatherGod, I didn't make any assumption about the code base. I took that information from @tacaswell's earlier reply, in these precise words, "Currently the backends are essentially independent and it is a nightmare to try to do any work on the interface (because you need to touch every backend)". Better argue that with @tacaswell, not me.

@liuyu81
Copy link
liuyu81 commented Mar 25, 2014

@WeatherGod, And I still believe you are asking to much from an external contributor to "not only contribute additional functionality, but also refactor the existing codebase to maximize code reuse".

@liuyu81
Copy link
liuyu81 commented Mar 25, 2014

@WeatherGod, Plus, don't you think it is more respectful to make add-on's, rather than write-over's, when contributing code to someone else's project?

@tacaswell
Copy link
Member

I know you should not feed trolls, but...

To be clear, what I meant was that the gtk_, gtk3_, tk_, macosx, wx_, and qt4* (I think that is all the gui backends we have left) families of backends are more-or-less independent (but most of them use the Agg backend for the actually rendering). Adding another family (qt5*) which could be folded into an existing family is the concern.

@mdboom
Copy link
Member
mdboom commented Mar 25, 2014

@liuyu81: Let's try and remain civil, please. We appreciate any and all contributions, however, those of us with more experience with the code base and the kinds of problems that have arisen in the past and a good sense of the desired future directions, can and should provide suggestions and criteria for how something is implemented. The original contribution is a good one, and I certainly appreciate the work involved. However, given the past experience with these things, it's clear that creating a new backend by copying-and-pasting should be avoided whenever possible, and this is one of those cases where it is very possible.

I believe there is consensus about how best to proceed here -- to create the Qt5 backend using a combinations of interface adaptors (as we already do for PySide and PyQt4) and class inheritance, rather than a wholesale copy. I think we should try to keep the discussion to constructive technical details about how best to proceed with that plan.

@liuyu81
Copy link
liuyu81 commented Mar 25, 2014

@tacaswell, @mdboom, I do understand that the maintenance burden is real -- a huge one. And I do understand -- after years of development -- the core team knows what's best for the project in the long run. What I am suggesting here, is that maybe you could to be nicer to external contributors -- shepherd them to refine their code, draw standard and setup concrete examples -- not just leave them alone until total satisfaction. This, I believe, is what's best for the community after all.

@WeatherGod
Copy link
Member

@liuyu81 , if you feel that we have not been as effective as possible in
helping potential contributors along in a PR, then we have obviously failed
you. We always try to give the right amount of encouragement and guidance
to outside contributors, and in reviewing this PR, I could not immediately
see how we could have done better (with the possible exception of more
timely responses). If it is your perception that we did not do our best to
encourage this contribution, then let's discuss (with specific examples)
how we could have done better. We can even make it a separate github issue
about this, if you would like.

@liuyu81
Copy link
liuyu81 commented Mar 26, 2014

@WeatherGod

Please excuse me for my straightforwardness.

As I see in this long discussion, people are deeming this PR as copy-and-paste (@mdboom and @tacaswell), and only changes import names (@Tillsten and @tacaswell).

Sounds like creating a perfect Qt5-backend that maximizes code reuse is only a piece of cake for those with more experience with the code base.

Well, it is perhaps true.

Problem is, the more experienced people are so busy that they can only specify their preferences. Then they just sit back and wait for the external contributor to get it right. Worse still, many of their comments are given in an arbitrary and criticism tone. For example,

  • @tacaswell: We will get the qt5 stuff in eventually, I don't really see the urgency yet (sounds to me like deeming the contributor's work worthless and of the lowest importance whatsoever);
  • @mdboom: I much prefer to adapt the namespace to be able to support all of them from a single code base (sounds to me like a critic demanding total satisfaction, not a grateful maintainer of an open-source project);

What I don't feel right, is that they would rather wait for 6 months, than do anything concrete to make the solution real. For end users, this is another 6 months without Qt5 support. And for the external contributor, this is another 6 months not getting recognized for his efforts.

To be honest, this is far from the best possible experience any user / contributor would expect.

@mfitzp
Copy link
Member
mfitzp commented Mar 26, 2014

@liuyu81

This isn't going to take me 6 months, I'd guess 6 weeks at worst. I'd do it
sooner but I'm currently in Austria attempting to learn to ski (quite
badly).

I've volunteered to do this because it scratches my own itch, but it is
important when contributing to open-source projects to ensure your
contributions don't create more work for the project your contributing to.
That's just common sense (and good manners).

What you characterise as the people experienced with the codebase as "only
specifying preferences" is actually them passing on their experience with
the codebase. As one of the outside contributors this is hugely useful to
ensure the code I produce works, fits the codebase, gets maintained and
continues to work for as long as possible. This in turn ensures it gets
merged as quickly as possible, and you get to use it.

We can have those discussions before I start or after I finish, but we're
going to have to have them.

If you absolutely can't wait the "working code" is there for you to
download (see my comment above for this with added mouse event support).
It's incomplete, and in no state to merge to matplotlib (my words, not the
matplotlib developers), but it's there. Just download, stick it in your
project folder and import it. If you're having trouble doing that then ask.

Finally, you're not being "straightforward" you're being rude. Being
unpleasant to the people you're relying on for help is not the way to get
results.

For the record I'm completely happy with the feedback received on my
adapted version of @badders code.
On 26 Mar 2014 11:58, "LIU Yu" notifications@github.com wrote:

@WeatherGod https://github.com/WeatherGod

Please excuse me for my straightforwardness.

As I see in this long discussion, people are deeming this PR as
copy-and-paste (@mdboom https://github.com/mdboom and @tacaswellhttps://github.com/tacaswell),
and only changes import names (@Tillsten https://github.com/Tillstenand
@tacaswell https://github.com/tacaswell). Sounds like creating a
perfect Qt5-backend that maximizes code reuse is only a piece of cake
for those with more experience with the code base.

Well, it is perhaps true.

Problem is, the more experienced people are so busy that they can only
specify their preferences. Then they just sit back and wait for the
external contributor to get it right. Worse still, many of their comments
are given in an arbitrary and criticism tone. For example,

We will get the qt5 stuff in eventually, I don't really see the
urgency yet
(sounds to me like deeming the contributor's work
worthless and of the lowest importance whatsoever);

I much prefer to adapt the namespace to be able to support all of
them from a single code base
(sounds to me like a critic demanding
total satisfaction, not a grateful maintainer of an open-source project);

What I don't feel right, is that they would rather wait for 6 months, than
do anything concrete to make the solution real. For end users, this is
another 6 months without Qt5 support. And for the external contributor,
this is another 6 months not getting recognized for his efforts.

To be honest, this is far from the best possible experience any user /
contributor would expect.

Reply to this email directly or view it on GitHubhttps://github.com//pull/2471#issuecomment-38670668
.

@liuyu81
Copy link
liuyu81 commented Mar 26, 2014

@mfitzp. I agree with you in that "passing on experience through specifying preferences" can be a viable approach to shepherding external contributors. But given this particular case, it has been proven to be an inefficient approach (one that took 6 months and still hasn't fruited). I would ask, is it absolutely necessary to stick to this "passing on experience" method to shepherd PR's? In other words, couldn't there be more effective, and perhaps more nurturing, ways to help external contributors come up with better solutions, sooner?

About me "being rude and unpleasant to the people I am relying on for help", I am sorry you feel it that way, but I am seeing this from a different perspective. The core developers -- without doubt -- contributed the significant part of current code base, and made available a wonderful product for free. In this particular case, however, they were not being as positive and effective as I -- an outsider -- would expect. As I see it, albeit for good reasons, these are the very people putting on hold a useful solution for 6 months. If only one of the experienced developers had offered more concrete help on this PR (rather than just passing on experiences), it is quite possible that Qt5 support would make into the code base months ago.

That said, I can't help to feel that there is still room to smooth PR shepherding for potential, future contributors. If I have been rude in expressing this feeling, that is NOT because I myself need Qt5 support that desperately -- I am not. And I took on this discussion, in part because it is suggested by @WeatherGod, and, more importantly, because it can be beneficial to the community after all.

@erikhvatum
Copy link

Hi Liu,

I want Qt5 support, but I am unbelievably, disreputably lazy (like all good
engineers), and everything I do is done so that I have to do less.

At first glance, the laziest and therefore best way to support Qt5 is to
update the existing Qt backend for Qt5 and tell all Qt4 users to stay on
the last version supporting Qt4 until they grow old and die. However,
matplotlib would probably lose a lot of users and contributors if it did
that, and I value the contributors because they do work so that I don't
have to.

Adding Qt5 support to the Qt4 codebase entails merging QtWidgets back into
QtGui, and generally seems like it will be a continual annoyance up until
everyone can finally agree that Qt4 is no longer relevant and the codebase
can finally start using Qt5 directly. Although initially attractive from
a lazyness perspective, it seems unlazy when I consider it.

If I weren't already insanely over-committed, I'd volunteer to take up
mftizp's code, buff out rough edges, and add a *Qt4 support shim *that
makes QtWidgets an alias to QtGui. Some day, years from now, someone will
realize that the Qt4 support shim stopped working at some point due to
Qt5-centric features being used - and that nobody noticed for months,
meaning that Qt4 support had become totally unimportant and dropped through
no conscious decision.

This is good because 1) it totally avoids needing to have an argument about
keeping/dropping support until it's a forgone conclusion anyway 2) we're
not working through a forward compatibility layer and 3) there's no big
changeover to new stuff, but instead a quite fading away of old stuff,
without requiring separate Qt4 and Qt5 backends being kept in sync (which
sounds suspiciously like actual work to me).

-Erik

On Wed, Mar 26, 2014 at 1:24 PM, LIU Yu notifications@github.com wrote:

@mfitzp https://github.com/mfitzp. I am grateful that you came forward
and volunteered to create a better (in terms of maintanence cost) Qt5
backend.

I agree with you in that "passing on experience through specifying
preferences" can be a viable approach to shepherding external contributors.
But given this particular issue, it has been proven to be an inefficient
approach (one that took 6 months and still hasn't fruited). I would ask, is
it absolutely necessary to stick to this "passing on experience" method to
shepherd PR's? In other words, couldn't there be more effective (and
perhaps more nurturing) ways to help external contributors come up with
better solutions, sooner?

About me "being rude and unpleasant to the people I am relying on for
help", I am sorry you feel it that way, but I am seeing this from a
different angle. The core developers -- without doubt -- contributed the
significant part of current code base, and made available a wonderful
product for free. For this particular issue, however, they are not being as
positive and effective as I -- an outsider -- would expect.

As I see it, albeit for good reasons, these are the very people putting on
hold a useful solution for 6 months. If only one of the experienced
developers had offered more concrete help on this PR (than just passing on
experiences), it is quite possible that Qt5 support would make into the
code base months ago. That said, I can't help to believe that there is
still room to smooth PR shepherding for potential, future contributors. And
I believe, this can be beneficial to the community after all.

Reply to this email directly or view it on GitHubhttps://github.com//pull/2471#issuecomment-38720986
.

@WeatherGod
Copy link
Member

@liuyu81, by @mfitzp's own admission, the code was not in a working state
for inclusion anyway. Many of us probably did not have access to a Qt5
environment to do our own testing and development to finish the work (I am
still on Qt4). Also, 6 months ago, the main concern dealt with confusion
over licensing (although they now seem to have been moot?). The work and
discussion on this issue really did not gain steam until a month ago, which
I would consider a fairly reasonable amount of time to spend on adding a
new backend and assessing its impacts on the codebase.

Now, I will grant you that -- as a general topic -- the matplotlib
developers could look into ways to more efficiently process PRs and issues.
Our release cycle is glacial compared to other open-source projects, and I
do worry about other projects usurping matplotlib's "throne" as the python
graphing library of choice. It could be an issue of developer "bandwidth",
or maybe one of insufficient documentation, or maybe community outreach. An
idea was tossed around at the last SciPy conference to use some grant money
to pay for a full or part time developer to alleviate some of the load on
the volunteers. Maybe we should revisit that idea?

@mfitzp
Copy link
Member
mfitzp commented Mar 27, 2014

@ erikhvatum This is coincidentally how I was intending to approach this -
shifting all the wrapping/compatibility to a separate import so the Qt
backends can get on with just doing what they do. Adding post-Qt5
compatibility will then not require the effort it does here, but backward
compatibility can be maintained.

@liuyu81 It hasn't "fruited" in 6 months because we're all busy, all
volunteers and this is more than a free-evenings work. You need to accept
that regardless of how important this is for you it's a low-priority for
everyone else. Most people are not yet using Qt5 (I'm only doing so because
of an incompatibility elsewhere) and there is a workaround that takes all
of 5 minutes to use. Your criticism of the matplotlib dev's feedback is
completely unfounded, not least because you're not the intended recipient
of it. I'm quite happy with what I've been asked to do, the only irritation
is your complaints on my "behalf".

If as @WeatherGod offered you want to discuss this further, open a separate
issue. This is a PR, the discussion should be about the code.
On 27 Mar 2014 16:59, "Erik Hvatum" notifications@github.com wrote:

Hi Liu,

I want Qt5 support, but I am unbelievably, disreputably lazy (like all good
engineers), and everything I do is done so that I have to do less.

At first glance, the laziest and therefore best way to support Qt5 is to
update the existing Qt backend for Qt5 and tell all Qt4 users to stay on
the last version supporting Qt4 until they grow old and die. However,
matplotlib would probably lose a lot of users and contributors if it did
that, and I value the contributors because they do work so that I don't
have to.

Adding Qt5 support to the Qt4 codebase entails merging QtWidgets back into
QtGui, and generally seems like it will be a continual annoyance up until
everyone can finally agree that Qt4 is no longer relevant and the codebase
can finally start using Qt5 directly. Although initially attractive from
a lazyness perspective, it seems unlazy when I consider it.

If I weren't already insanely over-committed, I'd volunteer to take up
mftizp's code, buff out rough edges, and add a *Qt4 support shim *that
makes QtWidgets an alias to QtGui. Some day, years from now, someone will
realize that the Qt4 support shim stopped working at some point due to
Qt5-centric features being used - and that nobody noticed for months,
meaning that Qt4 support had become totally unimportant and dropped through
no conscious decision.

This is good because 1) it totally avoids needing to have an argument about
keeping/dropping support until it's a forgone conclusion anyway 2) we're
not working through a forward compatibility layer and 3) there's no big
changeover to new stuff, but instead a quite fading away of old stuff,
without requiring separate Qt4 and Qt5 backends being kept in sync (which
sounds suspiciously like actual work to me).

-Erik

On Wed, Mar 26, 2014 at 1:24 PM, LIU Yu notifications@github.com wrote:

@mfitzp https://github.com/mfitzp. I am grateful that you came forward
and volunteered to create a better (in terms of maintanence cost) Qt5
backend.

I agree with you in that "passing on experience through specifying
preferences" can be a viable approach to shepherding external
contributors.
But given this particular issue, it has been proven to be an inefficient
approach (one that took 6 months and still hasn't fruited). I would ask,
is
it absolutely necessary to stick to this "passing on experience" method
to
shepherd PR's? In other words, couldn't there be more effective (and
perhaps more nurturing) ways to help external contributors come up with
better solutions, sooner?

About me "being rude and unpleasant to the people I am relying on for
help", I am sorry you feel it that way, but I am seeing this from a
different angle. The core developers -- without doubt -- contributed the
significant part of current code base, and made available a wonderful
product for free. For this particular issue, however, they are not being
as
positive and effective as I -- an outsider -- would expect.

As I see it, albeit for good reasons, these are the very people putting
on
hold a useful solution for 6 months. If only one of the experienced
developers had offered more concrete help on this PR (than just passing
on
experiences), it is quite possible that Qt5 support would make into the
code base months ago. That said, I can't help to believe that there is
still room to smooth PR shepherding for potential, future contributors.
And
I believe, this can be beneficial to the community after all.

Reply to this email directly or view it on GitHub<
https://github.com/matplotlib/matplotlib/pull/2471#issuecomment-38720986>
.

Reply to this email directly or view it on GitHubhttps://github.com//pull/2471#issuecomment-38822886
.

@liuyu81
Copy link
liuyu81 commented Mar 27, 2014

@WeatherGod thanks for the reply, and I really appreciate that you take it seriously.

@erikhvatum I quite like your idea of a backward-compatible Qt5 backend. If there isn't going to be one in foreseeable future, I will create one myself following this approach.

@mfitzp, I think I made it quite clear in my previous reply why I took on this discussion, and that Qt5-support was NOT a desperate need of mine (you missed the final paragraph of my comment, as it was not there in the email notification). And I don't see how -- in any of my previous comments -- I was expressing anything on your behalf. Throughout the discussion, my concern was about (if I should say) not-so-experienced external contributors, like @badders the creator of this PR. And I have never -- and I never thought I could -- express anything on behalf of any of the more-experienced developers such as you. That said, your irritation doesn't quite make sense to me.

Last but not least, I do agree that this general topic should be discussed under a separate issue. So I will back off on this. Good luck to the matplotlib project.

@mfitzp
Copy link
Member
mfitzp commented Mar 31, 2014

Good news!

I had a bit of down-time this evening and have managed to implement a PyQt5 backend based off the PyQt4 backend without copy-pasting. The implementation I've gone for is to use a small shim to make the PyQt5 API match that of PyQt4.

This would allow the same backend code to be used for both Qt4 and Qt5 if it wasn't for the changes to mouse event handling. To account for those a separate backend is defined that overrides the bit changed (wheelEvent). Unfortunately to do this there is a small change needed to the Qt4 backend to create a baseclass (for FigureCanvasQTAgg) but it has no effect on function.

There is one outstanding bug - updates (zoom, pan, etc.) don't correctly trigger redraw until the window is unfocused - but I'll work this out over the coming day. Once finished should I submit it as a new PR?

@tacaswell
Copy link
Member

@mfitzp Yes please. Although, @erikhvatum did make a compelling case for shimming the other way (making qt4 look like qt5).

I don't think we are going to get this in for 1.4. We don't have any automated tests for the gui code so it just needs to be exercised by hand, which means daily usage.

I am going to just have to restart #2624 from scratch.....

@mfitzp
Copy link
Member
mfitzp commented Apr 1, 2014

@tacaswell I've implemented it as "make Qt5 look like Qt4" to avoid doing very much to the Qt4 code (so only the Qt5 side of things needs 'testing'). But, for future-proofing going the other way would indeed make more sense. If there is no aversion to me altering the Qt4 side of things I'm happy to implement it that way.

Would you rather I held off until #2624 is complete?

@tacaswell
Copy link
Member

@mfitzp Ignore my whining.

As long as you don't break anything feel free to re-factor the existing code as much as you want.

@ejeschke
Copy link

Looking forward to this. I have Ginga supporting pyqt5, and I have a few plugins that use matplotlib.

@tacaswell tacaswell modified the milestones: v1.4.0, v1.5.x May 2, 2014
@tacaswell
Copy link
Member

ipython/ipython#5458 does this for ipython. I suspect we can closely follow it @mfitzp @badders

I am now thinking we should try to get this into 1.4.0....

@mfitzp
Copy link
Member
mfitzp commented May 18, 2014

@tacaswell I've commited a fully-working PyQt5 backend https://github.com/mfitzp/matplotlib with PyQt4 backends wrapping it to make changes as neccessary. I'm not able to test on Qt4 on my machine currently so if someone else could take a look that would be great. As far as I've tested, everything works in PyQt5.

In answer to your question re: IPython we've basically ended up doing this same thing. However, in their implementation they seem to be maintaining the Qt4 API (union of QtGui + QtWidgets) while this makes everything use the Qt5 (QtWidgets=QtGui).

@tacaswell
Copy link
Member

@mfitzp As a house keeping thing could you put it on a not-named-master branch and put in a new PR?

I only looked at it briefly and it looks good!

@mfitzp
Copy link
Member
mfitzp commented May 18, 2014

The pull request on branch backend-pyqt5 is open here:
#3072

On 18 May 2014 14:52, Thomas A Caswell notifications@github.com wrote:

@mfitzp https://github.com/mfitzp As a house keeping thing could you
put it on a not-named-master branch and put in a new PR?

I only looked at it briefly and it looks good!


Reply to this email directly or view it on GitHubhttps://github.com//pull/2471#issuecomment-43440241
.

@tacaswell
Copy link
Member

Closing in favor of #3072

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0