From e7f39aa2bb29eb3bf95d7c2ceee45522c709d4b8 Mon Sep 17 00:00:00 2001 From: tangkong Date: Fri, 26 Jul 2024 07:51:59 -0700 Subject: [PATCH 01/11] REF: move ICON_MAP to top-level widgets __init__ to avoid circular imports --- superscore/widgets/__init__.py | 16 ++++++++++++++++ superscore/widgets/page/__init__.py | 16 ++++------------ superscore/widgets/page/search.py | 2 +- superscore/widgets/window.py | 3 ++- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/superscore/widgets/__init__.py b/superscore/widgets/__init__.py index e69de29..46640fd 100644 --- a/superscore/widgets/__init__.py +++ b/superscore/widgets/__init__.py @@ -0,0 +1,16 @@ +def _get_icon_map(): + # do not pollute namespace + from superscore.model import Collection, Readback, Setpoint, Snapshot + + # a list of qtawesome icon names + icon_map = { + Collection: 'mdi.file-document-multiple', + Snapshot: 'mdi.camera', + Setpoint: 'mdi.target', + Readback: 'mdi.book-open-variant', + } + + return icon_map + + +ICON_MAP = _get_icon_map() diff --git a/superscore/widgets/page/__init__.py b/superscore/widgets/page/__init__.py index ca602a8..8f8a8dd 100644 --- a/superscore/widgets/page/__init__.py +++ b/superscore/widgets/page/__init__.py @@ -1,21 +1,13 @@ -def get_page_icon_map(): +def get_page_map(): # Don't pollute the namespace - from superscore.model import Collection, Readback, Setpoint, Snapshot + from superscore.model import Collection from superscore.widgets.page.entry import CollectionPage page_map = { Collection: CollectionPage } - # a list of qtawesome icon names - icon_map = { - Collection: 'mdi.file-document-multiple', - Snapshot: 'mdi.camera', - Setpoint: 'mdi.target', - Readback: 'mdi.book-open-variant', - } - - return page_map, icon_map + return page_map -PAGE_MAP, ICON_MAP = get_page_icon_map() +PAGE_MAP = get_page_map() diff --git a/superscore/widgets/page/search.py b/superscore/widgets/page/search.py index 02283f7..7dbe99e 100644 --- a/superscore/widgets/page/search.py +++ b/superscore/widgets/page/search.py @@ -9,8 +9,8 @@ from superscore.client import Client from superscore.model import Collection, Entry, Readback, Setpoint, Snapshot +from superscore.widgets import ICON_MAP from superscore.widgets.core import Display -from superscore.widgets.page import ICON_MAP logger = logging.getLogger(__name__) diff --git a/superscore/widgets/window.py b/superscore/widgets/window.py index c576c9f..34c6a45 100644 --- a/superscore/widgets/window.py +++ b/superscore/widgets/window.py @@ -11,8 +11,9 @@ from superscore.client import Client from superscore.model import Entry +from superscore.widgets import ICON_MAP from superscore.widgets.core import Display -from superscore.widgets.page import ICON_MAP, PAGE_MAP +from superscore.widgets.page import PAGE_MAP from superscore.widgets.page.search import SearchPage logger = logging.getLogger(__name__) From 134d73b0ae4602c0587c032956d58aeb69441fce Mon Sep 17 00:00:00 2001 From: tangkong Date: Fri, 26 Jul 2024 07:57:15 -0700 Subject: [PATCH 02/11] MNT: add root method to _Backend base class, needed for top-level tree views --- superscore/backends/core.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/superscore/backends/core.py b/superscore/backends/core.py index d799582..a151312 100644 --- a/superscore/backends/core.py +++ b/superscore/backends/core.py @@ -4,7 +4,7 @@ from typing import Generator from uuid import UUID -from superscore.model import Entry +from superscore.model import Entry, Root class _Backend: @@ -43,3 +43,8 @@ def update_entry(self, entry: Entry) -> None: def search(self, **search_kwargs) -> Generator[Entry, None, None]: """Yield a Entry objects corresponding matching ``search_kwargs``""" raise NotImplementedError + + @property + def root(self) -> Root: + """Return the Root Entry in this backend""" + raise NotImplementedError From d2b2e3095bda3c8f235f95c81a7c9bea48681f0d Mon Sep 17 00:00:00 2001 From: tangkong Date: Fri, 26 Jul 2024 08:00:18 -0700 Subject: [PATCH 03/11] ENH: add icons to TreeModel --- superscore/widgets/tree.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/superscore/widgets/tree.py b/superscore/widgets/tree.py index f66e790..e27471b 100644 --- a/superscore/widgets/tree.py +++ b/superscore/widgets/tree.py @@ -8,12 +8,14 @@ from typing import Any, ClassVar, Generator, List, Optional from weakref import WeakValueDictionary +import qtawesome as qta from PyQt5.QtCore import Qt from qtpy import QtCore from superscore.client import Client from superscore.model import Entry, Nestable, Root from superscore.qt_helpers import QDataclassBridge +from superscore.widgets import ICON_MAP logger = logging.getLogger(__name__) @@ -180,6 +182,13 @@ def takeChildren(self) -> list[EntryItem]: return children + def icon(self): + """return icon for this item""" + icon_id = ICON_MAP.get(type(self._data), None) + if icon_id is None: + return + return qta.icon(ICON_MAP[type(self._data)]) + def build_tree(entry: Entry, parent: Optional[EntryItem] = None) -> EntryItem: """ @@ -402,7 +411,7 @@ def data(self, index: QtCore.QModelIndex, role: int) -> Any: if role == Qt.DisplayRole: return item.data(1) if role == Qt.TextAlignmentRole: - return Qt.AlignCenter + return Qt.AlignLeft if role == Qt.ToolTipRole: return item.tooltip() @@ -412,4 +421,7 @@ def data(self, index: QtCore.QModelIndex, role: int) -> Any: if role == Qt.UserRole: return item + if role == Qt.DecorationRole and index.column() == 0: + return item.icon() + return None From a75211aea76581969d61b7a0615f1c57448c44d0 Mon Sep 17 00:00:00 2001 From: tangkong Date: Fri, 26 Jul 2024 08:02:28 -0700 Subject: [PATCH 04/11] ENH: add root tree view to main window, add context menu for opening page --- superscore/widgets/window.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/superscore/widgets/window.py b/superscore/widgets/window.py index 34c6a45..9a08243 100644 --- a/superscore/widgets/window.py +++ b/superscore/widgets/window.py @@ -15,6 +15,7 @@ from superscore.widgets.core import Display from superscore.widgets.page import PAGE_MAP from superscore.widgets.page.search import SearchPage +from superscore.widgets.tree import RootTree logger = logging.getLogger(__name__) @@ -44,6 +45,13 @@ def setup_ui(self) -> None: tab_bar.setElideMode(QtCore.Qt.ElideNone) self.tab_widget.tabCloseRequested.connect(self.tab_widget.removeTab) + # 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) + def open_page(self, entry: Entry) -> None: """ Open a page for ``entry`` in a new tab. @@ -78,3 +86,20 @@ def open_page(self, entry: Entry) -> None: def open_search_page(self) -> None: page = SearchPage(client=self.client, open_page_slot=self.open_page) self.tab_widget.addTab(page, 'search') + + def _tree_context_menu(self, pos: QtCore.QPoint) -> None: + self.menu = QtWidgets.QMenu(self) + index: QtCore.QModelIndex = self.tree_view.indexAt(pos) + entry: Entry = index.internalPointer()._data + + if index is not None and index.data() is not None: + # WeakPartialMethodSlot may not be needed, menus are transient + def open(*_, **__): + self.open_page(entry) + + open_action = self.menu.addAction( + f'&Open Detailed {type(entry).__name__} page' + ) + open_action.triggered.connect(open) + + self.menu.exec_(self.tree_view.mapToGlobal(pos)) From 10c59292c97ba1ebc944e3a8779ca755c613302a Mon Sep 17 00:00:00 2001 From: tangkong Date: Fri, 26 Jul 2024 08:17:20 -0700 Subject: [PATCH 05/11] ENH: Add fill_uuid method to EntryItem for use in TreeModel --- superscore/widgets/tree.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/superscore/widgets/tree.py b/superscore/widgets/tree.py index e27471b..89300ba 100644 --- a/superscore/widgets/tree.py +++ b/superscore/widgets/tree.py @@ -6,6 +6,7 @@ import logging from typing import Any, ClassVar, Generator, List, Optional +from uuid import UUID from weakref import WeakValueDictionary import qtawesome as qta @@ -54,6 +55,11 @@ 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``""" + if isinstance(self._data, UUID) and client is not None: + self._data = list(client.search(uuid=self._data))[0] + def data(self, column: int) -> Any: """ Return the data for the requested column. @@ -406,6 +412,8 @@ def data(self, index: QtCore.QModelIndex, role: int) -> Any: return None item: EntryItem = index.internalPointer() # Gives original EntryItem + item.fill_uuids(client=self.client) + # special handling for status info if index.column() == 1: if role == Qt.DisplayRole: From 602d7eefb2bf459be6c1129972d6a3e707a01992 Mon Sep 17 00:00:00 2001 From: tangkong Date: Fri, 26 Jul 2024 08:21:30 -0700 Subject: [PATCH 06/11] DOC: pre-release notes --- .../69-gui_top_tree.rst | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 docs/source/upcoming_release_notes/69-gui_top_tree.rst diff --git a/docs/source/upcoming_release_notes/69-gui_top_tree.rst b/docs/source/upcoming_release_notes/69-gui_top_tree.rst new file mode 100644 index 0000000..c802742 --- /dev/null +++ b/docs/source/upcoming_release_notes/69-gui_top_tree.rst @@ -0,0 +1,25 @@ +69 gui_top_tree +############### + +API Breaks +---------- +- N/A + +Features +-------- +- Adds top-level tree to main window +- Adds icons to the TreeModel +- Adds fill_uuid method to TreeModel, which grabs data from client if possible +- Adds context menu on tree view for accessing custom actions. + +Bugfixes +-------- +- N/A + +Maintenance +----------- +- N/A + +Contributors +------------ +- tangkong From dd95fc7139255a2d1dab812cfc217717f183824a Mon Sep 17 00:00:00 2001 From: tangkong Date: Fri, 26 Jul 2024 08:32:25 -0700 Subject: [PATCH 07/11] TST: let MagicMock truly do magic for mock_backend --- superscore/tests/conftest.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/superscore/tests/conftest.py b/superscore/tests/conftest.py index d30ff04..70c9b69 100644 --- a/superscore/tests/conftest.py +++ b/superscore/tests/conftest.py @@ -703,12 +703,8 @@ def dummy_cl() -> ControlLayer: @pytest.fixture(scope='function') def mock_backend() -> _Backend: - bk = _Backend() - bk.delete_entry = MagicMock() - bk.save_entry = MagicMock() - bk.get_entry = MagicMock() - bk.search = MagicMock() - bk.update_entry = MagicMock() + mock_bk = MagicMock(spec=_Backend) + return mock_bk class MockTaskStatus: From 64fcd6e07fd25533da5af3a6edf865cc6994b80a Mon Sep 17 00:00:00 2001 From: tangkong Date: Fri, 26 Jul 2024 09:25:56 -0700 Subject: [PATCH 08/11] TST: make test_page fixture chain simpler. Only create page in its test --- superscore/tests/test_page.py | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/superscore/tests/test_page.py b/superscore/tests/test_page.py index 23b63a1..a32d958 100644 --- a/superscore/tests/test_page.py +++ b/superscore/tests/test_page.py @@ -1,13 +1,10 @@ """Largely smoke tests for various pages""" -from typing import List - import pytest from pytestqt.qtbot import QtBot from superscore.client import Client from superscore.model import Collection -from superscore.widgets.core import DataWidget from superscore.widgets.page.entry import CollectionPage from superscore.widgets.page.search import SearchPage @@ -27,21 +24,7 @@ def search_page(qtbot: QtBot, mock_client: Client): return page -@pytest.fixture(scope='function') -def test_pages( - collection_page: CollectionPage, - search_page: SearchPage, -) -> List[DataWidget]: - return [collection_page, search_page,] - - -@pytest.fixture(scope='function') -def pages(request, test_pages: List[DataWidget]): - i = request.param - return test_pages[i] - - -@pytest.mark.parametrize('pages', [0, 1], indirect=True) -def test_page_smoke(pages): +@pytest.mark.parametrize('page', ["collection_page", "search_page"]) +def test_page_smoke(page: str, request: pytest.FixtureRequest): """smoke test, just create each page and see if they fail""" - print(type(pages)) + print(type(request.getfixturevalue(page))) From 00091433efd3f3affe01f778a7104ba48f0a8723 Mon Sep 17 00:00:00 2001 From: tangkong Date: Fri, 26 Jul 2024 10:40:33 -0700 Subject: [PATCH 09/11] TST: add test_apply_filter for search_page --- superscore/tests/conftest.py | 12 ++++++++++++ superscore/tests/test_page.py | 31 +++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/superscore/tests/conftest.py b/superscore/tests/conftest.py index 70c9b69..8bb730f 100644 --- a/superscore/tests/conftest.py +++ b/superscore/tests/conftest.py @@ -720,3 +720,15 @@ def done(self): def mock_client(mock_backend: _Backend) -> Client: client = Client(backend=mock_backend) return client + + +@pytest.fixture(scope='function') +def sample_client( + filestore_backend: FilestoreBackend, + dummy_cl: ControlLayer +) -> Client: + """Return a client with actula data, but no communication capabilities""" + client = Client(backend=filestore_backend) + client.cl = dummy_cl + + return client diff --git a/superscore/tests/test_page.py b/superscore/tests/test_page.py index a32d958..03139f2 100644 --- a/superscore/tests/test_page.py +++ b/superscore/tests/test_page.py @@ -18,8 +18,8 @@ def collection_page(qtbot: QtBot): @pytest.fixture(scope='function') -def search_page(qtbot: QtBot, mock_client: Client): - page = SearchPage(client=mock_client) +def search_page(qtbot: QtBot, sample_client: Client): + page = SearchPage(client=sample_client) qtbot.addWidget(page) return page @@ -28,3 +28,30 @@ def search_page(qtbot: QtBot, mock_client: Client): def test_page_smoke(page: str, request: pytest.FixtureRequest): """smoke test, just create each page and see if they fail""" print(type(request.getfixturevalue(page))) + + +def test_apply_filter(search_page: SearchPage): + search_page.apply_filter_button.clicked.emit() + assert search_page.results_table_view.model().rowCount() == 6 + + search_page.snapshot_checkbox.setChecked(False) + search_page.apply_filter_button.clicked.emit() + assert search_page.results_table_view.model().rowCount() == 5 + + search_page.readback_checkbox.setChecked(False) + search_page.apply_filter_button.clicked.emit() + assert search_page.results_table_view.model().rowCount() == 2 + + search_page.setpoint_checkbox.setChecked(False) + search_page.apply_filter_button.clicked.emit() + assert search_page.results_table_view.model().rowCount() == 1 + + # reset and try name filter + for box in search_page.type_checkboxes: + box.setChecked(True) + search_page.apply_filter_button.clicked.emit() + assert search_page.results_table_view.model().rowCount() == 6 + + search_page.name_line_edit.setText('collection 1') + search_page.apply_filter_button.clicked.emit() + assert search_page.results_table_view.model().rowCount() == 1 From 753aa9591aea643fd37a4d54736094dce2c73cf6 Mon Sep 17 00:00:00 2001 From: tangkong Date: Fri, 26 Jul 2024 10:42:43 -0700 Subject: [PATCH 10/11] BUG: provide default invalid index for TreeModel.index --- superscore/widgets/tree.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superscore/widgets/tree.py b/superscore/widgets/tree.py index 89300ba..f547547 100644 --- a/superscore/widgets/tree.py +++ b/superscore/widgets/tree.py @@ -286,7 +286,7 @@ def index( self, row: int, column: int, - parent: QtCore.QModelIndex = None + parent: QtCore.QModelIndex = QtCore.QModelIndex() ) -> QtCore.QModelIndex: """ Returns the index of the item in the model. From f52a53a5a19f45aea45996014954df4d3381f1fc Mon Sep 17 00:00:00 2001 From: tangkong Date: Fri, 26 Jul 2024 10:43:26 -0700 Subject: [PATCH 11/11] TST: add window tree_view manipulation test --- superscore/tests/test_window.py | 44 +++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/superscore/tests/test_window.py b/superscore/tests/test_window.py index 1654166..2d6d75f 100644 --- a/superscore/tests/test_window.py +++ b/superscore/tests/test_window.py @@ -1,10 +1,54 @@ +from uuid import UUID + from pytestqt.qtbot import QtBot from superscore.client import Client from superscore.widgets.window import Window +def count_visible_items(tree_view): + count = 0 + index = tree_view.model().index(0, 0) + while index.isValid(): + count += 1 + print(type(index.internalPointer()._data).__name__) + index = tree_view.indexBelow(index) + return count + + def test_main_window(qtbot: QtBot, mock_client: Client): """Pass if main window opens successfully""" window = Window(client=mock_client) qtbot.addWidget(window) + + +def test_sample_window(qtbot: QtBot, sample_client: Client): + window = Window(client=sample_client) + qtbot.addWidget(window) + + assert count_visible_items(window.tree_view) == 4 + + def get_last_index(index): + curr_index = index + while curr_index.isValid(): + new_index = window.tree_view.indexBelow(curr_index) + if not new_index.isValid(): + break + curr_index = new_index + return curr_index + + first_index = window.tree_view.model().index(0, 0) + last_index = get_last_index(first_index) + window.tree_view.expand(last_index) + + assert count_visible_items(window.tree_view) == 7 + + # get new last index after expansion, and signal view has been updated + new_last_index = get_last_index(first_index) + window.tree_view.dataChanged(first_index, new_last_index) + + # check that all exposed entries have been filled + index = first_index + while index.isValid(): + assert not isinstance(index.internalPointer()._data, UUID) + index = window.tree_view.indexBelow(index)