Recents menu (#39)

Moves load config to its own top-level button, and add a dropdown to it
that keeps recent items. Recent items are saved to a machine-wide config
(via QSettings) and can be pinned and assigned hotkeys for loading. Adds
unit tests for saving and loading recents.

Other changes
- Add model validation when loading yamls
- Cleaning up of tests

Author: ducky64

.github/workflows/python-app.yml

@@ -25,7 +25,8 @@ jobs:
         python-version: "3.10"
     - name: Install dependencies
       run: |
-        sudo apt-get install -y libgl1 libegl1 libxkbcommon-x11-0 libdbus-1-3
+        sudo apt-get update
+        sudo apt-get install --no-install-recommends -y libgl1 libegl1 libxkbcommon-x11-0 libdbus-1-3
         pip install -e .[dev]
         pip freeze  # dump debug info on packages
     - name: Check Black style

pyqtgraph_scope_plots/csv/csv_plots.py

@@ -24,7 +24,7 @@
 import pyqtgraph as pg
 import yaml
 from PySide6 import QtWidgets
-from PySide6.QtCore import QKeyCombination, QTimer
+from PySide6.QtCore import QKeyCombination, QTimer, QSettings
 from PySide6.QtGui import QAction, Qt
 from PySide6.QtWidgets import (
     QWidget,
@@ -36,7 +36,7 @@
     QToolButton,
     QMessageBox,
 )
-from pydantic import BaseModel
+from pydantic import BaseModel, ValidationError
 
 from ..legend_plot_widget import LegendPlotWidget
 from ..xy_plot_legends import XyTableLegends
@@ -177,18 +177,30 @@ def set_thickness(self, thickness: float) -> None:
             xy_plot.set_thickness(thickness)
 
 
+class CsvLoaderRecents(BaseModel):
+    hotkeys: Dict[int, str] = {}  # hotkey number -> file
+    recents: List[str] = []  # most recent first
+
+
 class CsvLoaderPlotsTableWidget(AnimationPlotsTableWidget, PlotsTableWidget, HasSaveLoadDataConfig):
     """Example app-level widget that loads CSV files into the plotter"""
 
     _MODEL_BASES = [CsvLoaderStateModel]
 
     WATCH_INTERVAL_MS = 333  # polls the filesystem metadata for changes this frequently
+    _RECENTS_MAX = 9  # hotkeys + recents is pruned to this count
+    _RECENTS_CONFIG_KEY = "recents"  # for QSettings
 
     _PLOT_TYPE = FullPlots
     _TABLE_TYPE = FullSignalsTable
 
+    @classmethod
+    def _config(cls) -> QSettings:  # for unit testability
+        return QSettings("scope-plots", "csv")
+
     def __init__(self, x_axis: Optional[Callable[[], pg.AxisItem]] = None) -> None:
         self._x_axis = x_axis
+        self._loaded_config_abspath = ""  # of last loaded config file, even if it has changed
 
         super().__init__()
 
@@ -267,6 +279,29 @@ def _make_controls(self) -> QWidget:
         button_load.setArrowType(Qt.ArrowType.DownArrow)
         button_load.setMenu(menu_load)
 
+        button_load_config = QToolButton()
+        button_load_config.setText("Load Config")
+        button_load_config.setToolButtonStyle(Qt.ToolButtonStyle.ToolButtonTextOnly)
+        button_load_config.setSizePolicy(QtWidgets.QSizePolicy.Policy.Preferred, QtWidgets.QSizePolicy.Policy.Fixed)
+        button_load_config.clicked.connect(self._on_load_config)
+
+        self._menu_config = QMenu(self)
+        button_load_config.setPopupMode(QToolButton.ToolButtonPopupMode.MenuButtonPopup)
+        button_load_config.setArrowType(Qt.ArrowType.DownArrow)
+        button_load_config.setMenu(self._menu_config)
+        self._menu_config.aboutToShow.connect(self._populate_config_menu)
+
+        self._load_hotkey_actions = []
+        for i in range(10):
+            load_hotkey_action = QAction(f"", self)
+            load_hotkey_action.setShortcut(
+                QKeyCombination(Qt.KeyboardModifier.ControlModifier, Qt.Key(Qt.Key.Key_0 + i))
+            )
+            load_hotkey_action.setShortcutContext(Qt.ShortcutContext.WidgetWithChildrenShortcut)
+            load_hotkey_action.triggered.connect(partial(self._load_hotkey_slot, i))
+            self.addAction(load_hotkey_action)
+            self._load_hotkey_actions.append(load_hotkey_action)
+
         button_refresh = QToolButton()
         button_refresh.setText("Refresh CSV")
         button_refresh.setToolButtonStyle(Qt.ToolButtonStyle.ToolButtonTextOnly)
@@ -308,21 +343,81 @@ def _make_controls(self) -> QWidget:
         button_menu.addAction(animation_action)
         button_visuals.setMenu(button_menu)
 
-        save_config_action = QAction("Save Config", button_menu)
-        save_config_action.triggered.connect(self._on_save_config)
-        button_menu.addAction(save_config_action)
-        load_config_action = QAction("Load Config", button_menu)
-        load_config_action.triggered.connect(self._on_load_config)
-        button_menu.addAction(load_config_action)
-
         layout = QVBoxLayout()
         layout.addWidget(button_load)
+        layout.addWidget(button_load_config)
         layout.addWidget(button_refresh)
         layout.addWidget(button_visuals)
         widget = QWidget()
         widget.setLayout(layout)
         return widget
 
+    def _get_recents(self) -> CsvLoaderRecents:
+        recents_val = cast(str, self._config().value(self._RECENTS_CONFIG_KEY, ""))
+        try:
+            return CsvLoaderRecents.model_validate(CsvLoaderRecents(**yaml.load(recents_val, Loader=TupleSafeLoader)))
+        except (yaml.YAMLError, TypeError, ValidationError):
+            return CsvLoaderRecents()
+
+    def _populate_config_menu(self) -> None:
+        self._menu_config.clear()
+        save_config_action = QAction("Save Config", self._menu_config)
+        save_config_action.triggered.connect(self._on_save_config)
+        self._menu_config.addAction(save_config_action)
+
+        self._menu_config.addSeparator()
+        recents = self._get_recents()
+        for hotkey, recent in sorted(recents.hotkeys.items(), key=lambda x: x[0]):
+            load_hotkey_action = self._load_hotkey_actions[hotkey]  # crash on invalid index
+            load_hotkey_action.setText(f"{os.path.split(recent)[1]}")
+            self._menu_config.addAction(load_hotkey_action)
+
+        for recent in recents.recents:
+            load_action = QAction(f"{os.path.split(recent)[1]}", self._menu_config)
+            load_action.triggered.connect(partial(self.load_config_file, recent))
+            self._menu_config.addAction(load_action)
+
+        self._menu_config.addSeparator()
+        set_hotkey_action = QAction("Set Hotkey", self._menu_config)
+        set_hotkey_action.triggered.connect(self._on_set_hotkey)
+        if self._loaded_config_abspath:
+            set_hotkey_action.setText(f"Set Hotkey for {os.path.split(self._loaded_config_abspath)[1]}")
+        else:
+            set_hotkey_action.setDisabled(True)
+        self._menu_config.addAction(set_hotkey_action)
+
+    def _on_set_hotkey(self) -> None:
+        assert self._loaded_config_abspath  # shouldn't be triggerable unless something loaded
+        recents = self._get_recents()
+
+        hotkey, ok = QInputDialog.getInt(self, "Set Hotkey Slot", "", value=0, minValue=0, maxValue=9)
+        if not ok:
+            return
+
+        if self._loaded_config_abspath in recents.recents:
+            recents.recents.remove(self._loaded_config_abspath)
+        recents.hotkeys[hotkey] = self._loaded_config_abspath
+        self._config().setValue(self._RECENTS_CONFIG_KEY, yaml.dump(recents.model_dump(), sort_keys=False))
+
+    def _load_hotkey_slot(self, slot: int) -> None:
+        recents = self._get_recents()
+        target = recents.hotkeys.get(slot, None)
+        if target is not None:
+            self.load_config_file(target)
+
+    def _append_recent(self) -> None:
+        recents = self._get_recents()
+        if self._loaded_config_abspath in recents.hotkeys.values():
+            return  # don't overwrite hotkeys
+        if self._loaded_config_abspath in recents.recents:
+            recents.recents.remove(self._loaded_config_abspath)
+        recents.recents.insert(0, self._loaded_config_abspath)
+        excess_recents = len(recents.recents) + len(recents.hotkeys) - self._RECENTS_MAX
+        if excess_recents > 0:
+            recents.recents = recents.recents[:-excess_recents]
+
+        self._config().setValue(self._RECENTS_CONFIG_KEY, yaml.dump(recents.model_dump(), sort_keys=False))
+
     def _on_load_csv(self) -> None:
         csv_filenames, _ = QFileDialog.getOpenFileNames(None, "Select CSV Files", filter="CSV files (*.csv)")
         if not csv_filenames:  # nothing selected, user canceled
@@ -476,21 +571,24 @@ def _do_save_config(self, filename: str) -> CsvLoaderStateModel:
             else:  # save as abspath, would need .. access to get CSVs
                 model.csv_files = [os.path.abspath(csv_filename) for csv_filename in self._csv_data_items.keys()]
 
+        self._loaded_config_abspath = os.path.abspath(filename)
+        self._append_recent()
+
         return model
 
     def _on_load_config(self) -> None:
         filename, _ = QFileDialog.getOpenFileName(None, "Load config", filter="YAML files (*.yml)")
         if not filename:  # nothing selected, user canceled
             return
+        self.load_config_file(filename)
+
+    def load_config_file(self, filename: str) -> None:
+        skeleton_model_type = self._create_skeleton_model_type()
         with open(filename, "r") as f:
-            model = self._parse_config(f)
+            model = skeleton_model_type.model_validate(skeleton_model_type(**yaml.load(f, Loader=TupleSafeLoader)))
+        assert isinstance(model, CsvLoaderStateModel)
         self._do_load_config(filename, model)
 
-    def _parse_config(self, f: Union[TextIO, str]) -> CsvLoaderStateModel:
-        loaded = self._create_skeleton_model_type()(**yaml.load(f, Loader=TupleSafeLoader))
-        assert isinstance(loaded, CsvLoaderStateModel)
-        return loaded
-
     def _do_load_config(self, filename: str, model: CsvLoaderStateModel) -> None:
         if model.csv_files is not None:
             missing_csv_files = []
@@ -521,3 +619,5 @@ def _do_load_config(self, filename: str, model: CsvLoaderStateModel) -> None:
         data_items = [(name, int_color(i), data_type) for i, (name, data_type) in enumerate(self._data_items.items())]
         self._set_data_items(data_items)
         self._set_data(data)  # bulk update everything for performance
+        self._loaded_config_abspath = os.path.abspath(filename)
+        self._append_recent()

tests/test_base_plot.py

@@ -23,7 +23,7 @@
 from pyqtgraph_scope_plots.multi_plot_widget import MultiPlotStateModel, PlotWidgetModel
 from pyqtgraph_scope_plots import MultiPlotWidget, PlotsTableWidget
 from .common_testdata import DATA_ITEMS, DATA
-from .test_util import assert_cast
+from .util import assert_cast
 
 
 @pytest.fixture()

tests/test_csv_viewer.py

@@ -17,9 +17,12 @@
 from unittest import mock
 
 import pytest
+from PySide6.QtCore import Qt
+from PySide6.QtWidgets import QInputDialog
 from pytestqt.qtbot import QtBot
 
-from pyqtgraph_scope_plots.csv.csv_plots import CsvLoaderPlotsTableWidget
+from pyqtgraph_scope_plots.csv.csv_plots import CsvLoaderPlotsTableWidget, CsvLoaderRecents
+from tests.util import MockQSettings, menu_action_by_name
 
 
 @pytest.fixture()
@@ -75,6 +78,7 @@ def test_watch_stability(qtbot: QtBot, plot: CsvLoaderPlotsTableWidget) -> None:
         qtbot.waitUntil(lambda: mock_load_csv.called)  # check the load happens
 
 
+@mock.patch.object(CsvLoaderPlotsTableWidget, "_config", lambda *args: MockQSettings())
 def test_save_model_csvs(qtbot: QtBot, plot: CsvLoaderPlotsTableWidget) -> None:
     # test empty save
     model = plot._do_save_config(os.path.join(os.path.dirname(__file__), "config.yml"))
@@ -93,6 +97,7 @@ def test_save_model_csvs(qtbot: QtBot, plot: CsvLoaderPlotsTableWidget) -> None:
     ]
 
 
+@mock.patch.object(CsvLoaderPlotsTableWidget, "_config", lambda *args: MockQSettings())
 def test_load_model_csvs_relpath(qtbot: QtBot, plot: CsvLoaderPlotsTableWidget) -> None:
     model = plot._do_save_config("/config.yml")
 
@@ -114,3 +119,80 @@ def test_load_model_csvs_relpath(qtbot: QtBot, plot: CsvLoaderPlotsTableWidget)
         mock_load_csv.assert_called_with(
             [os.path.join(os.path.dirname(__file__), "data", "test_csv_viewer_data.csv")], update=False
         )
+
+
+def test_recents_save(qtbot: QtBot, plot: CsvLoaderPlotsTableWidget) -> None:
+    settings = MockQSettings()
+    with mock.patch.object(CsvLoaderPlotsTableWidget, "_config", lambda *args: settings):
+        assert plot._get_recents() == CsvLoaderRecents()
+        model = plot._do_save_config("/config.yml")  # stores to recents
+        assert plot._get_recents().recents == [os.path.abspath("/config.yml")]
+
+        plot._do_load_config(os.path.join(os.path.dirname(__file__), "test.yml"), model)
+        assert plot._get_recents().recents == [
+            os.path.abspath(os.path.join(os.path.dirname(__file__), "test.yml")),
+            os.path.abspath("/config.yml"),
+        ]
+
+        # check reordering latest-first + dedup
+        plot._do_load_config("/config.yml", model)
+        assert plot._get_recents().recents == [
+            os.path.abspath("/config.yml"),
+            os.path.abspath(os.path.join(os.path.dirname(__file__), "test.yml")),
+        ]
+
+        # check pruning
+        for i in range(10):
+            plot._do_load_config(f"/extra{i}.yml", model)
+        assert len(plot._get_recents().recents) == 9
+
+        # check hotkeys
+        with mock.patch.object(QInputDialog, "getInt", lambda *args, **kwargs: (8, True)):
+            plot._on_set_hotkey()
+        assert plot._get_recents().hotkeys[8] == os.path.abspath(f"/extra9.yml")
+        assert len(plot._get_recents().recents) == 8
+        assert os.path.abspath(f"/extra9.yml") not in plot._get_recents().recents
+        assert os.path.abspath(f"/extra8.yml") in plot._get_recents().recents
+
+        # check most recent pruned
+        plot._do_load_config(f"/extra11.yml", model)
+        assert plot._get_recents().hotkeys[8] == os.path.abspath(f"/extra9.yml")
+        assert len(plot._get_recents().recents) == 8
+        assert os.path.abspath(f"/extra0.yml") not in plot._get_recents().recents
+        assert os.path.abspath(f"/extra11.yml") in plot._get_recents().recents
+
+        # check that it is possible to max out the hotkey range
+        for i in range(10):
+            plot._do_load_config(f"/extra{i}.yml", model)
+            with mock.patch.object(QInputDialog, "getInt", lambda *args, **kwargs: (i, True)):
+                plot._on_set_hotkey()
+        assert len(plot._get_recents().recents) == 0
+
+        # recents no longer saves, hotkeys take priority
+        plot._do_load_config(f"/extra11.yml", model)
+        assert len(plot._get_recents().recents) == 0
+
+
+@mock.patch.object(CsvLoaderPlotsTableWidget, "load_config_file")
+def test_recents_load(mock_load_config: mock.MagicMock, qtbot: QtBot, plot: CsvLoaderPlotsTableWidget) -> None:
+    settings = MockQSettings()
+    with mock.patch.object(CsvLoaderPlotsTableWidget, "_config", lambda *args: settings):
+        plot._populate_config_menu()
+        assert len(plot._menu_config.actions()) == 4  # 2 empty items + separators
+
+        qtbot.keyClick(plot, Qt.Key.Key_4, modifier=Qt.KeyboardModifier.ControlModifier)  # check nothing happens
+
+        plot._do_save_config("/config.yml")  # stores to recents
+        plot._populate_config_menu()
+        assert len(plot._menu_config.actions()) == 5
+        load_action = menu_action_by_name(plot._menu_config, "config.yml")
+        assert load_action is not None
+        load_action.trigger()
+        mock_load_config.assert_called_once_with(os.path.abspath("/config.yml"))
+        mock_load_config.reset_mock()
+
+        # set and test hotkey
+        with mock.patch.object(QInputDialog, "getInt", lambda *args, **kwargs: (4, True)):
+            plot._on_set_hotkey()
+        qtbot.keyClick(plot, Qt.Key.Key_4, modifier=Qt.KeyboardModifier.ControlModifier)
+        mock_load_config.assert_called_once_with(os.path.abspath("/config.yml"))

tests/test_transforms.py

@@ -25,7 +25,7 @@
 from pyqtgraph_scope_plots.transforms_signal_table import TransformsDataStateModel
 from pyqtgraph_scope_plots.util.util import not_none
 from .common_testdata import DATA
-from .test_util import context_menu, menu_action_by_name
+from .util import context_menu, menu_action_by_name
 
 
 @pytest.fixture()

tests/test_util.py renamed to tests/util.py

@@ -12,7 +12,8 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 
-from typing import TypeVar, Type, Any
+from typing import TypeVar, Type, Any, Optional, Dict
+from unittest import mock
 
 from PySide6.QtCore import Qt, QPoint
 from PySide6.QtGui import QAction
@@ -41,7 +42,7 @@ def context_menu(qtbot: QtBot, container: QWidget, target: QPoint = QPoint(0, 0)
     return assert_cast(QMenu, container.findChild(QMenu))
 
 
-def menu_action_by_name(menu: QMenu, *text: str) -> QAction:
+def menu_action_by_name(menu: QMenu, *text: str) -> Optional[QAction]:
     """Given a menu, returns the first action that contains the specified text, case-insensitive"""
     for i, current_text in enumerate(text):
         item_found = False
@@ -53,5 +54,19 @@ def menu_action_by_name(menu: QMenu, *text: str) -> QAction:
                 else:  # final step, return the action
                     return action
         if not item_found:
-            raise ValueError(f"No menu item with {current_text} in {[action.text() for action in menu.actions()]}")
-    raise ValueError()  # shouldn't happen, here to satisfy the type checker
+            return None
+    return None  # shouldn't happen, here to satisfy the type checker
+
+
+class MockQSettings(mock.MagicMock):
+    def __init__(self, *args: Any, settings_values: Optional[Dict[str, Any]] = None, **kwargs: Any) -> None:
+        super().__init__(*args, **kwargs)
+        self._values: Dict[str, Any] = {}
+        if settings_values:
+            self._values.update(settings_values)
+
+    def value(self, key: str, default: Any = None) -> Optional[Any]:
+        return self._values.get(key, default)
+
+    def setValue(self, key: str, value: Any) -> None:
+        self._values[key] = value