From 2bd4b237bab298a3e0cf279dacf125867a532ec7 Mon Sep 17 00:00:00 2001 From: GuiMacielPereira Date: Wed, 29 May 2024 16:03:35 +0100 Subject: [PATCH 1/8] Add key shortcuts 'k' and 'l' for axis scale Disabled the key shortcuts for changing the axis scale in matplotlib by default. Introduced a key press event so that keys 'k' and 'l' are now handled by us. The axis to be updated is picked from the position of the cursor. Made this change because previous default behaviour from matplotlib was unreliable, and it makes more sense to update axis in the same way we update them through the dialogs. - If multiple plots, only plot with the cursor on top of it will be updated - Colorfills work as expected, colorbar does not get updated - 3D plots are ignored - Normal mpl plots (non MantidAxis) will have default handling overwritten. --- .../workbench/workbench/plotting/config.py | 4 +++ .../workbench/plotting/figureinteraction.py | 25 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/qt/applications/workbench/workbench/plotting/config.py b/qt/applications/workbench/workbench/plotting/config.py index fad5cc92e6f6..e76f3381bf29 100644 --- a/qt/applications/workbench/workbench/plotting/config.py +++ b/qt/applications/workbench/workbench/plotting/config.py @@ -39,6 +39,10 @@ def initialize_matplotlib(): mpl.rcParams["figure.dpi"] = QApplication.instance().desktop().physicalDpiX() # Hide warning made by matplotlib before checking our backend. warnings.filterwarnings("ignore", message="Starting a Matplotlib GUI outside of the main thread will likely fail.") + # Disabling default key shortcuts for toggling axes scale + mpl.rcParams["keymap.xscale"].remove("k") + mpl.rcParams["keymap.xscale"].remove("L") + mpl.rcParams["keymap.yscale"].remove("l") def init_mpl_gcf(): diff --git a/qt/applications/workbench/workbench/plotting/figureinteraction.py b/qt/applications/workbench/workbench/plotting/figureinteraction.py index 10281cdd6ffe..15f8998fc712 100644 --- a/qt/applications/workbench/workbench/plotting/figureinteraction.py +++ b/qt/applications/workbench/workbench/plotting/figureinteraction.py @@ -57,6 +57,8 @@ ("Log x/Lin y", ("log", "linear")), ] ) +# Options to quickly switch axes scales with key shortcuts +AXES_SCALE_DEFAULT_SWITCH = ["linear", "log"] COLORBAR_SCALE_MENU_OPTS = OrderedDict([("Linear", Normalize), ("Log", LogNorm)]) @@ -101,6 +103,7 @@ def __init__(self, fig_manager): self._cids.append(canvas.mpl_connect("resize_event", self.mpl_redraw_annotations)) self._cids.append(canvas.mpl_connect("figure_leave_event", self.on_leave)) self._cids.append(canvas.mpl_connect("scroll_event", self.on_scroll)) + self._cids.append(canvas.mpl_connect("key_press_event", self.on_key_press)) self.canvas = canvas self.toolbar_manager = ToolbarStateManager(self.canvas.toolbar) @@ -144,6 +147,28 @@ def on_scroll(self, event): self.redraw_annotations() event.canvas.draw() + def on_key_press(self, event): + + ax = event.inaxes + if ax is None or isinstance(ax, Axes3D) or len(ax.images) == 0 and len(ax.lines) == 0: + return + + if event.key == "k": + current_xscale = ax.get_xscale() + next_xscale = self._get_next_axis_scale(current_xscale) + self._quick_change_axes((next_xscale, ax.get_yscale()), ax) + + if event.key == "l": + current_yscale = ax.get_yscale() + next_yscale = self._get_next_axis_scale(current_yscale) + self._quick_change_axes((ax.get_xscale(), next_yscale), ax) + + def _get_next_axis_scale(self, current_scale): + available_scales = AXES_SCALE_DEFAULT_SWITCH + scale_index = available_scales.index(current_scale) if current_scale in available_scales else 0 + next_scale = available_scales[(scale_index + 1) % len(available_scales)] + return next_scale + def on_mouse_button_press(self, event): """Respond to a MouseEvent where a button was pressed""" # local variables to avoid constant self lookup From ee8c133a9e91a56edd7f0307ff0697deca86c594 Mon Sep 17 00:00:00 2001 From: GuiMacielPereira Date: Thu, 30 May 2024 12:05:09 +0100 Subject: [PATCH 2/8] Get axes images and lines through getter method Should stick to getter methods if possible. --- .../workbench/workbench/plotting/figureinteraction.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qt/applications/workbench/workbench/plotting/figureinteraction.py b/qt/applications/workbench/workbench/plotting/figureinteraction.py index 15f8998fc712..2d054322e9d8 100644 --- a/qt/applications/workbench/workbench/plotting/figureinteraction.py +++ b/qt/applications/workbench/workbench/plotting/figureinteraction.py @@ -150,7 +150,7 @@ def on_scroll(self, event): def on_key_press(self, event): ax = event.inaxes - if ax is None or isinstance(ax, Axes3D) or len(ax.images) == 0 and len(ax.lines) == 0: + if ax is None or isinstance(ax, Axes3D) or len(ax.get_images()) == 0 and len(ax.get_lines()) == 0: return if event.key == "k": From 90884799f1986d6d72f88597339c18c32fff1bc1 Mon Sep 17 00:00:00 2001 From: GuiMacielPereira Date: Thu, 30 May 2024 12:10:40 +0100 Subject: [PATCH 3/8] Add unit test Added unit test for checking that a 'k' key press changes the right axis. Added call() to list of expected calls since I introduced am mlp connection to a key press event. The line that adds a "fake_line" is necessary so that if statement inside on_key_press() doesn't prevent the rest of the function from executing. --- .../plotting/test/test_figureinteraction.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/qt/applications/workbench/workbench/plotting/test/test_figureinteraction.py b/qt/applications/workbench/workbench/plotting/test/test_figureinteraction.py index fa8324b62358..6e84784943ec 100644 --- a/qt/applications/workbench/workbench/plotting/test/test_figureinteraction.py +++ b/qt/applications/workbench/workbench/plotting/test/test_figureinteraction.py @@ -87,6 +87,7 @@ def test_construction_registers_handler_for_button_press_event(self): call("resize_event", interactor.mpl_redraw_annotations), call("figure_leave_event", interactor.on_leave), call("scroll_event", interactor.on_scroll), + call("key_press_event", interactor.on_key_press), ] fig_manager.canvas.mpl_connect.assert_has_calls(expected_call) self.assertEqual(len(expected_call), fig_manager.canvas.mpl_connect.call_count) @@ -723,6 +724,22 @@ def test_click_y_axes_tick_label_launches_y_axes_editor(self, mock_y_editor): mock_y_editor.assert_called_once() + def test_keyboard_shortcuts_switch_axes_scale(self): + key_press_event = self._create_mock_key_press_event("k") + key_press_event.inaxes.get_xscale.return_value = "linear" + key_press_event.inaxes.get_yscale.return_value = "log" + key_press_event.inaxes.get_xlim.return_value = (0, 100) + key_press_event.inaxes.get_ylim.return_value = (5, 10) + key_press_event.inaxes.get_lines.return_value = ["fake_line"] + fig_manager = MagicMock() + fig_manager.canvas = MagicMock() + interactor = FigureInteraction(fig_manager) + interactor.on_key_press(key_press_event) + key_press_event.inaxes.set_xscale.assert_called_once_with("log") + key_press_event.inaxes.set_yscale.assert_called_once_with("log") + key_press_event.inaxes.set_xlim.assert_called_once_with((0, 100)) + key_press_event.inaxes.set_ylim.assert_called_once_with((5, 10)) + # Private methods def _create_mock_fig_manager_to_accept_right_click(self): fig_manager = MagicMock() @@ -761,6 +778,11 @@ def _create_mock_double_left_click(self): type(mouse_event).dblclick = PropertyMock(return_value=True) return mouse_event + def _create_mock_key_press_event(self, key): + key_press_event = MagicMock(inaxes=MagicMock(spec=MantidAxes, collections=[], creation_args=[{}])) + type(key_press_event).key = PropertyMock(return_value=key) + return key_press_event + def _create_axes_for_axes_editor_test(self, mouse_over: str): ax = MagicMock() ax.xaxis.contains.return_value = (mouse_over == "xaxis", {}) From b5f247465f75366e3ff44af0630f2af6daa99f3d Mon Sep 17 00:00:00 2001 From: GuiMacielPereira Date: Thu, 30 May 2024 13:39:42 +0100 Subject: [PATCH 4/8] Add release note --- docs/source/release/v6.11.0/Workbench/Bugfixes/37438.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/source/release/v6.11.0/Workbench/Bugfixes/37438.rst diff --git a/docs/source/release/v6.11.0/Workbench/Bugfixes/37438.rst b/docs/source/release/v6.11.0/Workbench/Bugfixes/37438.rst new file mode 100644 index 000000000000..3c7781a314e4 --- /dev/null +++ b/docs/source/release/v6.11.0/Workbench/Bugfixes/37438.rst @@ -0,0 +1 @@ +- Fixed key shortcuts `k` and `l` in mantid plot for quickly switching between `linear` and `log` axis. From 8d98f359a49f267cb81ba4a4cda979c2cca5941b Mon Sep 17 00:00:00 2001 From: GuiMacielPereira Date: Thu, 30 May 2024 16:35:11 +0100 Subject: [PATCH 5/8] Add documentation table of shortcuts Checked keymaps table in the default rcParams in matplolib. Tested keymaps that work on Mantid and wrote a similar table in our documentation. --- .../06_formatting_plots.rst | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/docs/source/tutorials/mantid_basic_course/loading_and_displaying_data/06_formatting_plots.rst b/docs/source/tutorials/mantid_basic_course/loading_and_displaying_data/06_formatting_plots.rst index 9b4f25e97853..8b6f7baba773 100644 --- a/docs/source/tutorials/mantid_basic_course/loading_and_displaying_data/06_formatting_plots.rst +++ b/docs/source/tutorials/mantid_basic_course/loading_and_displaying_data/06_formatting_plots.rst @@ -100,6 +100,43 @@ File > Settings | | +Useful Key Shortcuts +==================== + +Mantid plots support multiple key shortcuts by default. +Please note especially the shortcuts `k` and `l`, which are useful for quickly switching between linear and log axes scales. + +.. list-table:: + :header-rows: 1 + + * - Action + - Key shortcuts + * - Toggle fullscreen + - f, ctrl+f + * - Reset to homme + - h, r + * - Go back to previous view + - c, backspace + * - Go forward to next view + - v + * - Pan + - p + * - Zoom + - o + * - Save + - s + * - Quit figure + - q, ctrl+w, cmd+w + * - Toggle major grids + - g + * - Toggle minor grids + - G + * - Switch x scale between log/linear + - k + * - Switch y scale between log/linear + - l + + **Other Plotting Documentation** * :ref:`02_scripting_plots` From b9d8dbdf9117c7e866358fb03d0d3801a279a500 Mon Sep 17 00:00:00 2001 From: Gui Maciel Pereira <80104863+GuiMacielPereira@users.noreply.github.com> Date: Fri, 31 May 2024 10:20:39 +0100 Subject: [PATCH 6/8] Fix typo in release note Co-authored-by: Jonathan Haigh <35813666+jhaigh0@users.noreply.github.com> --- docs/source/release/v6.11.0/Workbench/Bugfixes/37438.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/release/v6.11.0/Workbench/Bugfixes/37438.rst b/docs/source/release/v6.11.0/Workbench/Bugfixes/37438.rst index 3c7781a314e4..ef1d03049ae9 100644 --- a/docs/source/release/v6.11.0/Workbench/Bugfixes/37438.rst +++ b/docs/source/release/v6.11.0/Workbench/Bugfixes/37438.rst @@ -1 +1 @@ -- Fixed key shortcuts `k` and `l` in mantid plot for quickly switching between `linear` and `log` axis. +- Fixed key shortcuts `k` and `l` in mantid plot for quickly switching between `linear` and `log` scales. From 0f6fd373bcbfd52d9af7b1bb84eab9e73878f04f Mon Sep 17 00:00:00 2001 From: GuiMacielPereira Date: Fri, 31 May 2024 10:26:23 +0100 Subject: [PATCH 7/8] Simplify function to change axis Previous proposal was over-engineered with the idea of possibly extending the number of scales the axis could scale to. Since we're likely to just stick to linear and log, the function is now easier to understand and maintain. --- .../workbench/workbench/plotting/figureinteraction.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/qt/applications/workbench/workbench/plotting/figureinteraction.py b/qt/applications/workbench/workbench/plotting/figureinteraction.py index 2d054322e9d8..b95eacb64f9a 100644 --- a/qt/applications/workbench/workbench/plotting/figureinteraction.py +++ b/qt/applications/workbench/workbench/plotting/figureinteraction.py @@ -57,8 +57,6 @@ ("Log x/Lin y", ("log", "linear")), ] ) -# Options to quickly switch axes scales with key shortcuts -AXES_SCALE_DEFAULT_SWITCH = ["linear", "log"] COLORBAR_SCALE_MENU_OPTS = OrderedDict([("Linear", Normalize), ("Log", LogNorm)]) @@ -148,7 +146,6 @@ def on_scroll(self, event): event.canvas.draw() def on_key_press(self, event): - ax = event.inaxes if ax is None or isinstance(ax, Axes3D) or len(ax.get_images()) == 0 and len(ax.get_lines()) == 0: return @@ -164,10 +161,9 @@ def on_key_press(self, event): self._quick_change_axes((ax.get_xscale(), next_yscale), ax) def _get_next_axis_scale(self, current_scale): - available_scales = AXES_SCALE_DEFAULT_SWITCH - scale_index = available_scales.index(current_scale) if current_scale in available_scales else 0 - next_scale = available_scales[(scale_index + 1) % len(available_scales)] - return next_scale + if current_scale == "linear": + return "log" + return "linear" def on_mouse_button_press(self, event): """Respond to a MouseEvent where a button was pressed""" From c0cdaaf3222a2014437ec6327a8adede030dca86 Mon Sep 17 00:00:00 2001 From: GuiMacielPereira Date: Fri, 31 May 2024 10:31:29 +0100 Subject: [PATCH 8/8] Add unit test for `l` shortcut Since I already had a test for `k`, added a similar test for `l`. --- .../plotting/test/test_figureinteraction.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/qt/applications/workbench/workbench/plotting/test/test_figureinteraction.py b/qt/applications/workbench/workbench/plotting/test/test_figureinteraction.py index 6e84784943ec..2598c3bde695 100644 --- a/qt/applications/workbench/workbench/plotting/test/test_figureinteraction.py +++ b/qt/applications/workbench/workbench/plotting/test/test_figureinteraction.py @@ -724,7 +724,7 @@ def test_click_y_axes_tick_label_launches_y_axes_editor(self, mock_y_editor): mock_y_editor.assert_called_once() - def test_keyboard_shortcuts_switch_axes_scale(self): + def test_keyboard_shortcut_switch_x_scale(self): key_press_event = self._create_mock_key_press_event("k") key_press_event.inaxes.get_xscale.return_value = "linear" key_press_event.inaxes.get_yscale.return_value = "log" @@ -740,6 +740,22 @@ def test_keyboard_shortcuts_switch_axes_scale(self): key_press_event.inaxes.set_xlim.assert_called_once_with((0, 100)) key_press_event.inaxes.set_ylim.assert_called_once_with((5, 10)) + def test_keyboard_shortcut_switch_y_scale(self): + key_press_event = self._create_mock_key_press_event("l") + key_press_event.inaxes.get_xscale.return_value = "linear" + key_press_event.inaxes.get_yscale.return_value = "log" + key_press_event.inaxes.get_xlim.return_value = (0, 100) + key_press_event.inaxes.get_ylim.return_value = (5, 10) + key_press_event.inaxes.get_lines.return_value = ["fake_line"] + fig_manager = MagicMock() + fig_manager.canvas = MagicMock() + interactor = FigureInteraction(fig_manager) + interactor.on_key_press(key_press_event) + key_press_event.inaxes.set_xscale.assert_called_once_with("linear") + key_press_event.inaxes.set_yscale.assert_called_once_with("linear") + key_press_event.inaxes.set_xlim.assert_called_once_with((0, 100)) + key_press_event.inaxes.set_ylim.assert_called_once_with((5, 10)) + # Private methods def _create_mock_fig_manager_to_accept_right_click(self): fig_manager = MagicMock()