From 3094d3a8c44fa86e69bd932f070a57eb86bb0df2 Mon Sep 17 00:00:00 2001 From: tangkong Date: Mon, 18 Nov 2024 16:31:49 -0800 Subject: [PATCH 1/7] BUG: fill the first layer of children whenever a RootTree is initialized, ensuring children are valid --- superscore/tests/test_views.py | 25 +++++++++++++++---------- superscore/widgets/views.py | 2 ++ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/superscore/tests/test_views.py b/superscore/tests/test_views.py index e987e29..227aef5 100644 --- a/superscore/tests/test_views.py +++ b/superscore/tests/test_views.py @@ -242,24 +242,29 @@ def test_fill_uuids_entry_item(linac_backend: TestBackend, qtbot: QtBot): nested_coll.swap_to_uuids() assert all(isinstance(c, UUID) for c in nested_coll.children) - tree_model = RootTree(base_entry=nested_coll, client=client) + # hack: make all of linac_backend flat + for entry in linac_backend._entry_cache.values(): + entry.swap_to_uuids() + tree_model = RootTree(base_entry=nested_coll, client=client) original_depth = nest_depth(tree_model.root_item) - assert original_depth == 1 + # default fill depth is 2, so children and their children get EntryItems + assert original_depth == 2 # fill just the first child # fill depth can depend on how the backend returns data. Backend may not # be lazy, so we assert only child1's children have EntryItems root_item = tree_model.root_item child1 = root_item.child(0) - assert child1.childCount() == 0 + assert child1.child(0).childCount() == 0 child1.fill_uuids(client) - assert child1.childCount() > 0 - assert root_item.child(1).childCount() == 0 - assert root_item.child(2).childCount() == 0 + assert child1.child(0).childCount() > 0 + assert root_item.child(1).child(0).childCount() == 0 + assert root_item.child(2).child(0).childCount() == 0 - # filling the root item fills its children, which held uuids before + # filling only occurs if direct children are UUIDs, does nothing here + # since it was originally filled by RootTree root_item.fill_uuids(client) - assert root_item.child(0).childCount() > 0 - assert root_item.child(1).childCount() > 0 - assert root_item.child(2).childCount() > 0 + assert root_item.child(0).child(0).childCount() > 0 + assert root_item.child(1).child(0).childCount() == 0 + assert root_item.child(2).child(0).childCount() == 0 diff --git a/superscore/widgets/views.py b/superscore/widgets/views.py index e8a4332..1e3a096 100644 --- a/superscore/widgets/views.py +++ b/superscore/widgets/views.py @@ -288,6 +288,8 @@ def __init__( self.base_entry = base_entry self.root_item = build_tree(base_entry) self.client = client + # ensure at least the first set of children are filled + self.root_item.fill_uuids(self.client) self.headers = ['name', 'description'] def refresh_tree(self) -> None: From 965feafbbea40c00b7d131e5d68fc89dbc765bdf Mon Sep 17 00:00:00 2001 From: tangkong Date: Mon, 18 Nov 2024 16:37:59 -0800 Subject: [PATCH 2/7] REF: move tree-view context menu into new RootTreeView --- superscore/tests/test_window.py | 4 +- superscore/ui/main_window.ui | 11 +++- superscore/widgets/views.py | 109 ++++++++++++++++++++++++++++++++ superscore/widgets/window.py | 43 ++++++------- 4 files changed, 140 insertions(+), 27 deletions(-) diff --git a/superscore/tests/test_window.py b/superscore/tests/test_window.py index 2d057d6..b6fdc45 100644 --- a/superscore/tests/test_window.py +++ b/superscore/tests/test_window.py @@ -16,9 +16,9 @@ def count_visible_items(tree_view): return count -def test_main_window(qtbot: QtBot, mock_client: Client): +def test_main_window(qtbot: QtBot, sample_client: Client): """Pass if main window opens successfully""" - window = Window(client=mock_client) + window = Window(client=sample_client) qtbot.addWidget(window) diff --git a/superscore/ui/main_window.ui b/superscore/ui/main_window.ui index 0bce192..4f6e15b 100644 --- a/superscore/ui/main_window.ui +++ b/superscore/ui/main_window.ui @@ -53,7 +53,7 @@ - + 0 @@ -85,7 +85,7 @@ 0 0 800 - 37 + 20 @@ -177,6 +177,13 @@ + + + RootTreeView + QTreeView +
superscore.widgets.views
+
+
diff --git a/superscore/widgets/views.py b/superscore/widgets/views.py index 1e3a096..e71f3cb 100644 --- a/superscore/widgets/views.py +++ b/superscore/widgets/views.py @@ -7,6 +7,7 @@ import logging import time from enum import Enum, IntEnum, auto +from functools import partial from typing import Any, ClassVar, Dict, Generator, List, Optional, Union from uuid import UUID from weakref import WeakValueDictionary @@ -499,6 +500,114 @@ def fetchMore(self, parent: QtCore.QModelIndex) -> None: self.endInsertRows() +class RootTreeView(QtWidgets.QTreeView): + """ + Tree view for displaying an Entry. + Contains a standard context menu and action set + """ + + _model: Optional[RootTree] = None + data_updated: ClassVar[QtCore.Signal] = QtCore.Signal() + + def __init__( + self, + *args, + client: Optional[Client] = None, + entry: Optional[Entry] = None, + open_page_slot: Optional[OpenPageSlot] = None, + **kwargs, + ): + super().__init__(*args, **kwargs) + self._client = client + self.data = entry + self.open_page_slot = open_page_slot + + self.setup_ui() + + def setup_ui(self) -> None: + # Configure basic settings + self.maybe_setup_model() + + self.setContextMenuPolicy(QtCore.Qt.CustomContextMenu) + self.customContextMenuRequested.connect(self._tree_context_menu) + self.setExpandsOnDoubleClick(False) + self.doubleClicked.connect(self.open_index) + + @property + def client(self): + return self._client + + @client.setter + def client(self, client: Client): + self._set_client(client) + + def _set_client(self, client: Client): + if client is self._client: + return + + if not isinstance(client, Client): + raise ValueError("Provided client is not a superscore Client") + + self._client = client + self.maybe_setup_model() + + def set_data(self, data: Any): + """Set the data for this view, re-setup ui""" + if not isinstance(data, (Root, Entry)): + raise ValueError( + f"Attempted to set an incompatable data type ({type(data)})" + ) + self.data = data + self.maybe_setup_model() + + def maybe_setup_model(self): + if self.client is None: + logger.debug("Client not set, cannot initialize model") + return + + if self.data is None: + logger.debug("data not set, cannot initialize model") + return + + self._model = RootTree(base_entry=self.data, client=self.client) + self.setModel(self._model) + self._model.dataChanged.connect(self.data_updated) + + self.data_updated.emit() + + def _tree_context_menu(self, pos: QtCore.QPoint) -> None: + index: QtCore.QModelIndex = self.indexAt(pos) + if index is not None and index.data() is not None: + entry: Entry = index.internalPointer()._data + menu = self.create_context_menu(entry) + + menu.exec_(self.mapToGlobal(pos)) + + def create_context_menu(self, entry: Entry) -> QtWidgets.QMenu: + """ + Default method for creating the context menu. + Overload/replace this method if you would like to change this behavior + """ + menu = QtWidgets.QMenu(self) + open_action = menu.addAction( + f'&Open Detailed {type(entry).__name__} page' + ) + # WeakPartialMethodSlot may not be needed, menus are transient + open_action.triggered.connect(partial(self.open_page, entry)) + + return menu + + def open_index(self, index: QtCore.QModelIndex) -> None: + entry: Entry = index.internalPointer()._data + print(f'double click on : {type(entry)}') + self.open_page(entry) + + def open_page(self, entry): + """Simple wrapper around the open page slot""" + if self.open_page_slot is not None: + self.open_page_slot(entry) + + class HeaderEnum(IntEnum): """ Enum for more readable header names. Underscores will be replaced with spaces diff --git a/superscore/widgets/window.py b/superscore/widgets/window.py index 8dd27e8..084db0d 100644 --- a/superscore/widgets/window.py +++ b/superscore/widgets/window.py @@ -20,7 +20,7 @@ from superscore.widgets.page.collection_builder import CollectionBuilderPage from superscore.widgets.page.restore import RestorePage from superscore.widgets.page.search import SearchPage -from superscore.widgets.views import RootTree +from superscore.widgets.views import RootTreeView logger = logging.getLogger(__name__) @@ -30,7 +30,7 @@ class Window(Display, QtWidgets.QMainWindow): filename = 'main_window.ui' - tree_view: QtWidgets.QTreeView + tree_view: RootTreeView tab_widget: QtWidgets.QTabWidget action_new_coll: QtWidgets.QAction @@ -55,13 +55,11 @@ def setup_ui(self) -> None: self.tab_widget.tabCloseRequested.connect(self.remove_tab) # setup tree view - self.tree_model = RootTree(base_entry=self.client.backend.root, - client=self.client) - self.tree_view.setModel(self.tree_model) - self.tree_view.setContextMenuPolicy(QtCore.Qt.CustomContextMenu) - self.tree_view.customContextMenuRequested.connect(self._tree_context_menu) - self.tree_view.setExpandsOnDoubleClick(False) - self.tree_view.doubleClicked.connect(self.open_index) + self.tree_view.client = self.client + self.tree_view.set_data(self.client.backend.root) + # override context menu and open_page_slot methods + self.tree_view.create_context_menu = self._window_context_menu + self.tree_view.open_page_slot = self.open_page # setup actions self.action_new_coll.triggered.connect(self.open_collection_builder) @@ -144,20 +142,19 @@ def open_restore_page(self, snapshot: Snapshot) -> None: index = self.tab_widget.addTab(page, snapshot.title) self.tab_widget.setCurrentIndex(index) - def _tree_context_menu(self, pos: QtCore.QPoint) -> None: - self.menu = QtWidgets.QMenu(self) - index: QtCore.QModelIndex = self.tree_view.indexAt(pos) - if index is not None and index.data() is not None: - entry: Entry = index.internalPointer()._data - open_action = self.menu.addAction( - f'&Open Detailed {type(entry).__name__} page' - ) - # WeakPartialMethodSlot may not be needed, menus are transient - open_action.triggered.connect(partial(self.open_page, entry)) - if isinstance(entry, Snapshot): - restore_page_action = self.menu.addAction('Inspect values') - restore_page_action.triggered.connect(partial(self.open_restore_page, entry)) - self.menu.exec_(self.tree_view.mapToGlobal(pos)) + def _window_context_menu(self, entry: Entry) -> QtWidgets.QMenu: + """override for RootTreeView context menu""" + menu = QtWidgets.QMenu(self) + open_action = menu.addAction( + f'&Open Detailed {type(entry).__name__} page' + ) + # WeakPartialMethodSlot may not be needed, menus are transient + open_action.triggered.connect(partial(self.open_page, entry)) + if isinstance(entry, Snapshot): + restore_page_action = menu.addAction('Inspect values') + restore_page_action.triggered.connect(partial(self.open_restore_page, entry)) + + return menu def closeEvent(self, a0: QCloseEvent) -> None: while self.tab_widget.count() > 0: From f0f8e02e4ed02b952dad1e341e2ed9a83592bfd1 Mon Sep 17 00:00:00 2001 From: tangkong Date: Mon, 18 Nov 2024 17:02:45 -0800 Subject: [PATCH 3/7] REF: replace existing tree views with RootTreeView (CollectionBuilderPage, NestablePage) --- superscore/ui/collection_builder_page.ui | 7 ++++++- superscore/ui/nestable_page.ui | 7 ++++++- superscore/widgets/page/collection_builder.py | 11 +++++++---- superscore/widgets/page/entry.py | 10 ++++++---- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/superscore/ui/collection_builder_page.ui b/superscore/ui/collection_builder_page.ui index 58d5b31..e20e3c5 100644 --- a/superscore/ui/collection_builder_page.ui +++ b/superscore/ui/collection_builder_page.ui @@ -42,7 +42,7 @@ Qt::Horizontal - + 1 @@ -213,6 +213,11 @@ QTableView
superscore.widgets.views
+ + RootTreeView + QTreeView +
superscore.widgets.views
+
diff --git a/superscore/ui/nestable_page.ui b/superscore/ui/nestable_page.ui index bf05470..acf92ea 100644 --- a/superscore/ui/nestable_page.ui +++ b/superscore/ui/nestable_page.ui @@ -42,7 +42,7 @@ Qt::Horizontal - + 1 @@ -85,6 +85,11 @@ QTableView
superscore.widgets.views
+ + RootTreeView + QTreeView +
superscore.widgets.views
+
diff --git a/superscore/widgets/page/collection_builder.py b/superscore/widgets/page/collection_builder.py index 4d115c1..fd0d166 100644 --- a/superscore/widgets/page/collection_builder.py +++ b/superscore/widgets/page/collection_builder.py @@ -12,7 +12,7 @@ from superscore.widgets.manip_helpers import insert_widget from superscore.widgets.views import (BaseTableEntryModel, LivePVHeader, LivePVTableView, NestableTableView, - RootTree) + RootTree, RootTreeView) logger = logging.getLogger(__name__) @@ -24,7 +24,7 @@ class CollectionBuilderPage(Display, DataWidget): meta_placeholder: QtWidgets.QWidget meta_widget: NameDescTagsWidget - tree_view: QtWidgets.QTreeView + tree_view: RootTreeView sub_coll_table_view: NestableTableView sub_pv_table_view: LivePVTableView @@ -88,8 +88,11 @@ def setup_ui(self): self.sub_coll_table_view.client = self.client self.sub_coll_table_view.set_data(self.data) - self.tree_model = RootTree(base_entry=self.data, client=self.client) - self.tree_view.setModel(self.tree_model) + self.tree_view.client = self.client + self.tree_view.set_data(self.data) + self.tree_view.open_page_slot = self.open_page_slot + self.tree_model: RootTree = self.tree_view.model() + self.sub_coll_table_view.data_updated.connect(self.tree_model.refresh_tree) self.sub_pv_table_view.data_updated.connect(self.tree_model.refresh_tree) diff --git a/superscore/widgets/page/entry.py b/superscore/widgets/page/entry.py index c8c5d53..c6d37f9 100644 --- a/superscore/widgets/page/entry.py +++ b/superscore/widgets/page/entry.py @@ -19,7 +19,8 @@ match_line_edit_text_width) from superscore.widgets.thread_helpers import BusyCursorThread from superscore.widgets.views import (LivePVTableView, NestableTableView, - RootTree, edit_widget_from_epics_data) + RootTreeView, + edit_widget_from_epics_data) logger = logging.getLogger(__name__) @@ -29,7 +30,7 @@ class NestablePage(Display, DataWidget): meta_placeholder: QtWidgets.QWidget meta_widget: NameDescTagsWidget - tree_view: QtWidgets.QTreeView + tree_view: RootTreeView sub_coll_table_view: NestableTableView sub_pv_table_view: LivePVTableView @@ -58,8 +59,9 @@ def setup_ui(self): insert_widget(self.meta_widget, self.meta_placeholder) # show tree view - self.model = RootTree(base_entry=self.data, client=self.client) - self.tree_view.setModel(self.model) + self.tree_view.client = self.client + self.tree_view.set_data(self.data) + self.tree_view.open_page_slot = self.open_page_slot self.sub_pv_table_view.client = self.client self.sub_pv_table_view.set_data(self.data) From 3c9519210bc3c4d3c7965228f4e4c496b8466f2d Mon Sep 17 00:00:00 2001 From: tangkong Date: Mon, 18 Nov 2024 17:07:39 -0800 Subject: [PATCH 4/7] TST: mark TestBackend as not a test, so pytest stops trying to collect it --- superscore/backends/test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/superscore/backends/test.py b/superscore/backends/test.py index 71b9d13..9e0a042 100644 --- a/superscore/backends/test.py +++ b/superscore/backends/test.py @@ -13,6 +13,7 @@ class TestBackend(_Backend): """Backend that manipulates Entries in-memory, for testing purposes.""" + __test__ = False # Tell pytest this isn't a test case _entry_cache: Dict[UUID, Entry] = {} def __init__(self, data: Optional[List[Entry]] = None): From d9bde0cc88c424633d45dfcbe97ea96e5e41ea1d Mon Sep 17 00:00:00 2001 From: tangkong Date: Mon, 18 Nov 2024 17:23:13 -0800 Subject: [PATCH 5/7] TST: add som RootTreeView setup tests, move an existing one around --- superscore/tests/test_views.py | 33 +++++++++++++++++++++++++++++--- superscore/tests/test_widgets.py | 11 +---------- 2 files changed, 31 insertions(+), 13 deletions(-) diff --git a/superscore/tests/test_views.py b/superscore/tests/test_views.py index 227aef5..bb3e248 100644 --- a/superscore/tests/test_views.py +++ b/superscore/tests/test_views.py @@ -11,11 +11,12 @@ from superscore.backends.test import TestBackend from superscore.client import Client from superscore.control_layers import EpicsData -from superscore.model import Collection, Parameter, Severity, Status +from superscore.model import Collection, Parameter, Root, Severity, Status from superscore.tests.conftest import nest_depth -from superscore.widgets.views import (CustRoles, LivePVHeader, +from superscore.widgets.views import (CustRoles, EntryItem, LivePVHeader, LivePVTableModel, LivePVTableView, - NestableTableView, RootTree) + NestableTableView, RootTree, + RootTreeView) @pytest.fixture(scope='function') @@ -268,3 +269,29 @@ def test_fill_uuids_entry_item(linac_backend: TestBackend, qtbot: QtBot): assert root_item.child(0).child(0).childCount() > 0 assert root_item.child(1).child(0).childCount() == 0 assert root_item.child(2).child(0).childCount() == 0 + + +def test_roottree_setup(sample_database: Root): + tree_model = RootTree(base_entry=sample_database) + root_index = tree_model.index_from_item(tree_model.root_item) + # Check that the entire tree was created + assert tree_model.rowCount(root_index) == 4 + assert tree_model.root_item.child(3).childCount() == 3 + + +def test_root_tree_view_setup_init_args(sample_client: Client): + tree_view = RootTreeView( + client=sample_client, + entry=sample_client.backend.root + ) + assert isinstance(tree_view.model().root_item, EntryItem) + assert isinstance(tree_view.model(), RootTree) + + +def test_root_tree_view_setup_post_init(sample_client: Client): + tree_view = RootTreeView() + tree_view.client = sample_client + tree_view.set_data(sample_client.backend.root) + + assert isinstance(tree_view.model().root_item, EntryItem) + assert isinstance(tree_view.model(), RootTree) diff --git a/superscore/tests/test_widgets.py b/superscore/tests/test_widgets.py index f3bd2f2..9e2be7f 100644 --- a/superscore/tests/test_widgets.py +++ b/superscore/tests/test_widgets.py @@ -5,9 +5,8 @@ import pytest from pytestqt.qtbot import QtBot -from superscore.model import Collection, Root +from superscore.model import Collection from superscore.widgets.core import DataWidget -from superscore.widgets.views import RootTree @pytest.mark.parametrize( @@ -39,11 +38,3 @@ def test_collection_datawidget_bridge( qtbot.addWidget(widget1) qtbot.addWidget(widget2) - - -def test_roottree_setup(sample_database: Root): - tree_model = RootTree(base_entry=sample_database) - root_index = tree_model.index_from_item(tree_model.root_item) - # Check that the entire tree was created - assert tree_model.rowCount(root_index) == 4 - assert tree_model.root_item.child(3).childCount() == 3 From 742d87c0da3e77901ec613fb848ff8980c53b3e6 Mon Sep 17 00:00:00 2001 From: tangkong Date: Mon, 18 Nov 2024 17:31:16 -0800 Subject: [PATCH 6/7] DOC: pre-release notes --- .../107-mnt_upstream_tree_view.rst | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 docs/source/upcoming_release_notes/107-mnt_upstream_tree_view.rst diff --git a/docs/source/upcoming_release_notes/107-mnt_upstream_tree_view.rst b/docs/source/upcoming_release_notes/107-mnt_upstream_tree_view.rst new file mode 100644 index 0000000..f0d1b1f --- /dev/null +++ b/docs/source/upcoming_release_notes/107-mnt_upstream_tree_view.rst @@ -0,0 +1,22 @@ +107 mnt_upstream_tree_view +########################## + +API Breaks +---------- +- N/A + +Features +-------- +- N/A + +Bugfixes +-------- +- Fills the top-level entry whenever RootTree model is created, to ensure at least first level of children display + +Maintenance +----------- +- Adds common RootTreeView which holds the RootTree model. Upstreams standard settings and context menu support + +Contributors +------------ +- tangkong From 4d3a24c8b761a6b12c06f4d02954d9c0bbda115f Mon Sep 17 00:00:00 2001 From: tangkong Date: Tue, 3 Dec 2024 10:29:35 -0800 Subject: [PATCH 7/7] MNT: adjust fill_uuids to take depth optional argument, consolidate client.setter method in RootTreeView --- superscore/widgets/views.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/superscore/widgets/views.py b/superscore/widgets/views.py index e71f3cb..710026b 100644 --- a/superscore/widgets/views.py +++ b/superscore/widgets/views.py @@ -71,8 +71,15 @@ def __init__( self._bridge_cache[id(data)] = bridge self.bridge = bridge - def fill_uuids(self, client: Optional[Client] = None) -> None: - """Fill this item's data if it is a uuid, using ``client``""" + def fill_uuids( + self, + client: Optional[Client] = None, + fill_depth: int = 2 + ) -> None: + """ + Fill this item's data if it is a uuid, using ``client``. + By default fills to a depth of 2, to keep the tree view data loading lazy + """ if client is None: return @@ -82,7 +89,7 @@ def fill_uuids(self, client: Optional[Client] = None) -> None: if isinstance(self._data, Nestable): if any(isinstance(child, UUID) for child in self._data.children): - client.fill(self._data, fill_depth=2) + client.fill(self._data, fill_depth=fill_depth) # re-construct child EntryItems if there is a mismatch or if any # hold UUIDs as _data @@ -539,9 +546,6 @@ def client(self): @client.setter def client(self, client: Client): - self._set_client(client) - - def _set_client(self, client: Client): if client is self._client: return