Skip to content

FIX: port the key handling changes from mpl - #310

Merged
martinRenou merged 1 commit into
matplotlib:masterfrom
tacaswell:fix_mpl34
Mar 19, 2021
Merged

martinRenou merged 1 commit into
matplotlib:masterfrom
tacaswell:fix_mpl34

Conversation

@tacaswell

Copy link
Copy Markdown
Member

ports changes from matplotlib/matplotlib#19146

closes #309

@tacaswell

Copy link
Copy Markdown
Member Author

Style checkers: 2 Tom: nil

Comment thread ipympl/backend_nbagg.py
def new_timer(self, *args, **kwargs):
return TimerTornado(*args, **kwargs)

if matplotlib.__version__ < '3.4':

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

woah neat! I had no idea this was a thing you can do

@ianhi

ianhi commented Mar 18, 2021

Copy link
Copy Markdown
Collaborator

👍 Makes sense to me.

It would also be great to have somewhere to put your test case #309 (comment) somewhere (especially for whenever a proper widget testing framework solidifies - which I think will be based on https://github.com/jupyterlab/galata ???)

A good place would be the UAT notebook from #305. But if this gets merged first lets just make sure to open a follow up issue for adding to the UAT.

@martinRenou

Copy link
Copy Markdown
Member

which I think will be based on https://github.com/jupyterlab/galata ???

That would be great indeed.

@tacaswell

Copy link
Copy Markdown
Member Author

I'm not sure what the current ipympl release schedule is, but can this go out as soon as possible? We are probably going to do mpl 3.4 in the next few days.

@ianhi

ianhi commented Mar 18, 2021

Copy link
Copy Markdown
Collaborator

I'm not sure what the current ipympl release schedule is, but can this go out as soon as possible? We are probably going to do mpl 3.4 in the next few days.

I think the release schedule has been: Something gets merged, then we wait a few days until someone remembers to make a new release. So lets release right after merging this. (Or maybe after this + #305)

@ianhi

ianhi commented Mar 18, 2021

Copy link
Copy Markdown
Collaborator

I tried this out, but it's not actually clear to me what this fixes? Is it only possible to see the bug in a non-qwerty layout?

Overall I trust that this fixes the issue though

@tacaswell

Copy link
Copy Markdown
Member Author

Without this patch, but with mpl3.4.0rcX / v3.4.x / master when you type keys you get back the key code as a string. mpl3.3 and before we shipped back the key codes and then converted to keys a (private) method on the Python side. We overhauled that code for mpl34 which required changes to both the py and js sides. ipympl re-uses the Python side logic (did not trace it through fully, but it is via inheritance) but has its own version of the js side.

This PR back-ports the js side, so that we feed mpl34 the values it expects, but that would break ipypmpl with any, ah, currently released version of Matplotlib which seems bad. So this also backports the conversion function and hides it behind a mpl version-gated at class-definition time check to selectively over-ride the private method. Thus if we do change the private function again (without changing the protocol to with the js side), then ipympl will get the improvements and old versions of mpl still work :)

@martinRenou

Copy link
Copy Markdown
Member

Thanks!

@martinRenou
martinRenou merged commit e364c99 into matplotlib:master Mar 19, 2021
@tacaswell
tacaswell deleted the fix_mpl34 branch March 19, 2021 14:27
@ianhi

ianhi commented Mar 22, 2021

Copy link
Copy Markdown
Collaborator

This should be in version 0.7.0 on pypi. @martinRenou does anything extra need to happen to update the conda-forge version?

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.

incompatibility with mpl 3.4

3 participants