Repository navigation
Description
There is special casing for ctl / cmd in the Qt backend on OSX
matplotlib/lib/matplotlib/backends/backend_qt.py
Lines 49 to 51 in 9465538
| # In OSX, the control and super (aka cmd/apple) keys are switched. | |
| ("Key_Control", "control" if sys.platform != "darwin" else "cmd"), | |
| ("Key_Meta", "meta" if sys.platform != "darwin" else "control"), |
That is making the tests fail.
Looking at a US mac keyboard on the left side I see:
ctrl | option | command
and looking at US windows keyboard I see
ctrl | meta (aka windows) | alt
however that does not tell us what keycodes they actually generate.
We used to implement this logic differently (before we unified the qt backend code)
3.4.3
matplotlib/lib/matplotlib/backends/backend_qt5.py
Lines 62 to 67 in 919145f
| if sys.platform == 'darwin': | |
| # in OSX, the control and super (aka cmd/apple) keys are switched, so | |
| # switch them back. | |
| SPECIAL_KEYS.update({QtCore.Qt.Key_Control: 'cmd', # cmd/apple key | |
| QtCore.Qt.Key_Meta: 'control', | |
| }) |
3.2.2
matplotlib/lib/matplotlib/backends/backend_qt5.py
Lines 70 to 85 in a1a5298
| MODIFIER_KEYS = [('super', QtCore.Qt.MetaModifier, QtCore.Qt.Key_Meta), | |
| ('alt', QtCore.Qt.AltModifier, QtCore.Qt.Key_Alt), | |
| ('ctrl', QtCore.Qt.ControlModifier, QtCore.Qt.Key_Control), | |
| ('shift', QtCore.Qt.ShiftModifier, QtCore.Qt.Key_Shift), | |
| ] | |
| if sys.platform == 'darwin': | |
| # in OSX, the control and super (aka cmd/apple) keys are switched, so | |
| # switch them back. | |
| SPECIAL_KEYS.update({QtCore.Qt.Key_Control: 'cmd', # cmd/apple key | |
| QtCore.Qt.Key_Meta: 'control', | |
| }) | |
| MODIFIER_KEYS[0] = ('cmd', QtCore.Qt.ControlModifier, | |
| QtCore.Qt.Key_Control) | |
| MODIFIER_KEYS[2] = ('ctrl', QtCore.Qt.MetaModifier, | |
| QtCore.Qt.Key_Meta) |
This swapping goes all the way back to 5fdd68a (2012).
I think we need to:
- have a mac user with a mac keyboard veryify that the keys they hit match the keycodes we expect
- if the keys are right to the human typing then fix the tests (probably by tweaking what the key-codes fed in are)
- if the keys are wrong to the human typing
- if they are consistent across mpl versions on osx: gnash our teeth in despair and figure out how we want to fix this (on one hand, we should make sure that the key-codes are consistent across platfroms, on the other hand we do not want to break existing OSX users).
- if they changed from 3.4 to the default branch, fix the re-mapping
I also have a worry that Qt has started to fix this for us.
FYI: I just ran the latest tests and these are now failing for me and I'm wondering if it has to do with being on a mac? The tests look like they are swapping cmd/ctrl, so I tried swapping the modifier keys on my system and that didn't help.
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[Qt5Agg-control] - AssertionError: assert 'cmd+a' == 'ctrl+a'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[Qt5Agg-alt_control] - AssertionError: assert 'alt+cmd' == 'alt+control'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[Qt5Agg-control_alt] - AssertionError: assert 'cmd+alt' == 'ctrl+alt'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[Qt5Agg-modifier_order] - AssertionError: assert 'cmd+alt+ctrl+á' == 'ctrl+alt+super+á'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[Qt5Agg-backspace_mod] - AssertionError: assert 'cmd+backspace' == 'ctrl+backspace'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[QtAgg-control] - AssertionError: assert 'cmd+a' == 'ctrl+a'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[QtAgg-alt_control] - AssertionError: assert 'alt+cmd' == 'alt+control'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[QtAgg-control_alt] - AssertionError: assert 'cmd+alt' == 'ctrl+alt'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[QtAgg-modifier_order] - AssertionError: assert 'cmd+alt+ctrl+á' == 'ctrl+alt+super+á'
FAILED lib/matplotlib/tests/test_backend_qt.py::test_correct_key[QtAgg-backspace_mod] - AssertionError: assert 'cmd+backspace' == 'ctrl+backspace'It looks like these may be getting skipped on the macos-latest CI?
SKIPPED [22] lib/matplotlib/tests/test_backend_qt.py:168: No usable Qt bindingsOriginally posted by @greglucas in #20868 (comment)
Activity
dstansby commentedon Sep 18, 2021
https://doc.qt.io/qt-5/qt.html#Modifier-enum says:
so I think the failing tests make sense; as an example, with
qt_key = 'Key_A'andqt_mods = ['ControlModifier']we should be identifying this asctrl+A, apart from on macOS whereControlModifiermeans the Command key has been pressed. So the solution seems to be change the expected answer in the tests if run on macOS.tacaswell commentedon Sep 20, 2021
Expected answer or input? I would lean towards the input side as what we want is "the user hit the key that produces ctrl in our output" which from a user point of view is what we want to preserve.
timhoffm commentedon Sep 20, 2021
Ctrl on Mac is something different than Ctrl on other platforms, it just happens to have the same name. On a Mac, the equivalent to
CtrlisCmd(e.g.Cmd+Cis copy).Qt as a cross-platform framework has decided to map this internally, so that
Cmd+Cwill still give theQt::CTRLmodifier, which is reasonable because this way the application code does not have to distinguish between platforms.Without checking, I think our current behavior is
Cmd-C->Qt::CTRL+Qt::Key_C->'cmd-c'Ctrl-C->Qt::CTRL+Qt::Key_C->'ctrl-c'Since
test_backend_qt.py::test_correct_keyentries at the Qt level, we have to adapt the expected outputs to make the tests pass.A second question is whether this native remapping is reasonable or if we should behave like Qt. If platform independence is desireable, we could introduce a rcParam that allows to configure that. We cannot switch hard without breaking user code.