matplotlib
matplotlib copied to clipboard
Separately track modifier keys for mouse events.
PR Summary
Ensure that modifier keys are correctly reported for mouse events even if the last key press occurred out of the canvas.
Whether the event modifiers are directly available on enter/leave events depends on the backend, but all are handled here (except possibly for macos, which I haven't checked).
There's a first separate commit for the macosx backend for some preliminary work with ObjC.
Followup to #16931 (it's the reason for that big PR...). Supersedes #6159. See also the side note at https://github.com/matplotlib/matplotlib/issues/23436#issuecomment-1186302186.
PR Checklist
Tests and Styling
- [ ] Has pytest style unit tests (and
pytest
passes). - [ ] Is Flake 8 compliant (install
flake8-docstrings
and runflake8 --docstring-convention=all
).
Documentation
- [ ] New features are documented, with examples if plot related.
- [ ] New features have an entry in
doc/users/next_whats_new/
(follow instructions in README.rst there). - [ ] API changes documented in
doc/api/next_api_changes/
(follow instructions in README.rst there). - [ ] Documentation is sphinx and numpydoc compliant (the docs should build without error).
I haven't had a chance to look at the details here, but I just pulled and tested quick on macosx and hit some errors that you can probably fix pretty quick:
Clicking anywhere:
TypeError: MouseEvent.__init__() got an unexpected keyword argument 'num'
ctrl + clicking anywhere:
SystemError: NULL object passed to Py_BuildValue
Oops, thanks for trying. Hopefully fixed now?
Looks like the failures with macosx are real and a PROCESS_EVENT
snuck in after your initial PR.
Oops, probably a careless rebase, thanks for the ping.
Ah, I had forgotten to implement the exclude kwarg to _mpl_modifiers for some backends, thanks for catching that, the doubling of modifiers should now be fixed. Can I ask for a joker and defer adding modifiers to key events to another issue?
I suspect it's just a matter of updating (if needed) the table in _mpl_modifiers (previously _get_key) (perhaps by manually inspecting event.state in various cases); as you have access to a mac, can you give it a try (perhaps both with a laptop keyboard and with an independent one just to be sure)?
I tested qt + tk on linux and osx on osx.
Also saw something funny going on with 'meta' on tk. I saw it in the out put from the snippet Greg posted, but I could not sort out which keys I hit to get it.
Anyone can merge on green (again, excluding the patch coverage).
Thank you @anntzer !
@greglucas I got my hands on a mac :) and had a look at https://github.com/matplotlib/matplotlib/pull/23473#pullrequestreview-1216540516. I think the relevant patch to fix the super+meta issue on tk is basically
diff --git i/lib/matplotlib/backends/_backend_tk.py w/lib/matplotlib/backends/_backend_tk.py
index 7aa24e06ff..6893120ace 100644
--- i/lib/matplotlib/backends/_backend_tk.py
+++ w/lib/matplotlib/backends/_backend_tk.py
@@ -354,7 +354,7 @@ class FigureCanvasTk(FigureCanvasBase):
("ctrl", 1 << 2, "control"),
("alt", 1 << 4, "alt"),
("shift", 1 << 0, "shift"),
- ("super", 1 << 3, "super"),
+ ("foobar", 1 << 3, "meta"),
] if sys.platform == "darwin" else [
("ctrl", 1 << 2, "control"),
("alt", 1 << 3, "alt"),
where the last tuple item must be "meta" (to match tk reporting the cmd key with event.keysym = "Meta_L" (or "Meta_R")) and the first entry is how you think the modifier should actually be called (I guess "cmd", or perhaps "meta", I don't really have a good intuition here)? Can you decide on that last point and pick up the patch?