Skip to content

FIX: shim Qt4 and Qt5 together better - #9048

Closed
tacaswell wants to merge 1 commit into
matplotlib:masterfrom
tacaswell:fix_qt4_MI
Closed

tacaswell wants to merge 1 commit into
matplotlib:masterfrom
tacaswell:fix_qt4_MI

Conversation

@tacaswell

Copy link
Copy Markdown
Member

in two places super needs to be used in the Qt5 case and
frustrated in the Qt4 case. These are handled by the two
_fake_super_* methods. The must be just private as we need to
override in sub-classes.

Closes #9040

in two places `super` needs to be used in the Qt5 case and
frustrated in the Qt4 case.  These are handled by the two
`_fake_super_*` methods.  The must be just private as we need to
override in sub-classes.

Closes matplotlib#9040
@tacaswell tacaswell added this to the 2.1 (next point release) milestone Aug 17, 2017
@tacaswell tacaswell added Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. GUI: Qt labels Aug 17, 2017
@tacaswell
tacaswell requested review from anntzer and astrofrog August 17, 2017 03:35
@anntzer

anntzer commented Aug 17, 2017

Copy link
Copy Markdown
Contributor

Would the patch at https://github.com/matplotlib/matplotlib/pull/8771/files#diff-76b5bbaa0c6a702660db7feae68c1d20 (starting at "Work around lack of cooperative inheritance") solve the issue too?

@tacaswell

Copy link
Copy Markdown
Member Author

I am not sure if I am more comfortable with this approach or via re-implementing the MRO innards.

@dopplershift

Copy link
Copy Markdown
Contributor

If we only need to fix this for these classes, I say go with @tacaswell because there's less introspection going on (we've codified the special case in comments and the inheritance structure).

If we get to the point where we have lots of classes dealing with this nonsense, then I'd say the complexity of the solution in #8771 is warranted.

# Note different super-calling style to backend_qt5
QtWidgets.QWidget.__init__(self)
FigureCanvasBase.__init__(self, figure)
self.figure = figure

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why'd all this other stuff, like setMouseTracking go away?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that stuff is handled in the FigureCanvasQt5 init and this function injected in.

@anntzer

anntzer commented Aug 17, 2017

Copy link
Copy Markdown
Contributor

I think @tacaswell should just decide whether we want to try to get #8771 in any time soon or not, and decide accordingly for this PR.

@dopplershift

Copy link
Copy Markdown
Contributor

Not knowing the state of #8771, I'm not sure if it's good to merge something so large towards the end of the 2.1 release cycle, whereas this particular issue is probably a showstopper for 2.1.

@anntzer

anntzer commented Aug 17, 2017

Copy link
Copy Markdown
Contributor

See #9052 for the alternate approach, without the cairo parts which are irrelevant for this.

@tacaswell

Copy link
Copy Markdown
Member Author

I prefer @anntzer 's solution (see #9052 for reasons).

@fariza

fariza commented Aug 21, 2017

Copy link
Copy Markdown
Member

I confirm that this solves the problem with #9040

@tacaswell

Copy link
Copy Markdown
Member Author

@fariza Can you also test out #9052 ? I think we are going to go with that solution.

@tacaswell tacaswell closed this Aug 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GUI: Qt Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants