Replace reactions tree view with model/view table and switch metabolites to table widget - #571
Closed
axelvonkamp wants to merge 3 commits into
Closed
Replace reactions tree view with model/view table and switch metabolites to table widget#571axelvonkamp wants to merge 3 commits into
axelvonkamp wants to merge 3 commits into
Conversation
### Motivation - Replace the previous `QTreeWidget`-based implementation with a model/view approach to separate data and presentation and make the reaction list more maintainable and efficient. - Provide an explicit row data object and table model so coloring, tooltips, sorting and editing logic are encapsulated in the model layer. - Preserve the existing reactions-list behavior (dragging, selection, editing scenario cells, pinning, column hide/copy) while making future UI changes easier. ### Description - Added `ReactionListItem` as a lightweight row-data container that stores per-column text, background/foreground colors, tooltips, sort values, pin and hidden state, and emits change notifications to the model via `_emit_changed`. - Implemented `ReactionListModel` subclassing `QAbstractTableModel` to provide row/column counts, `data`/`setData`, `flags`, header text, insertion/removal (`add_item`/`takeTopLevelItem`/`clear`), and a `sort` implementation that keeps pinned rows at the top. - Replaced the previous `QTreeWidget` subclass with `DragableTableView` (a `QTableView` subclass) and a `ReactionListRootItem` adapter so existing code that iterates tree-like rows still works; updated `ReactionList` to instantiate and wire `ReactionListModel` + `DragableTableView` and to use model signals (`itemChanged`) instead of widget `itemChanged` events. - Kept external API/behavior compatible by providing adapter methods such as `findItems`, `takeTopLevelItem`, `indexOfTopLevelItem`, `setCurrentItem`, `scrollToItem`, `editItem`, and retaining context-menu, pin/unpin, coloring, clipboard export and scenario editing logic. ### Testing - Ran `python3 -m py_compile cnapy/gui_elements/reactions_list.py` which succeeded. - Ran `python3 -m py_compile cnapy/gui_elements/central_widget.py cnapy/gui_elements/reactions_list.py` which succeeded and verified the updated usage points compile cleanly. - Ran quick static checks (`git diff --check`) with no reported problems; attempted to import the module at runtime with `python -c "from cnapy.gui_elements.reactions_list import ReactionListModel, ReactionListColumn"` but the import failed due to the test environment missing system OpenGL library (`libGL.so.1`) which prevents Qt GUI initialization, so dynamic run-time GUI tests could not be executed in this CI environment.
…tes to table widget ### Motivation - Modernize the reactions list to a model/view design to improve performance and simplify item manipulation. - Remove direct dependence on `QTreeWidgetItem` and tree APIs to allow clearer sorting, hiding and selection handling. - Use a proper table for metabolites in the reaction mask to better present columns and enable stable item data storage. ### Description - Introduced `ReactionListModel` (subclass of `QAbstractTableModel`) and `ReactionListItem` as the row data object, and replaced the `QTreeWidget` with `DragableTableView` (a `QTableView` wrapper) for the reactions list UI. - Replaced many direct tree operations with model-based operations (`reaction_model.items`) and updated signal connections to use `ReactionListModel.itemChanged`. - Updated sorting, column header access and context menu code to use `horizontalHeader()` and table-based APIs, and adjusted blocking comments to reflect signal changes. - Implemented row hiding for search via `ReactionListItem.setHidden` and rewrote `update_selected` to use a compiled regex for matching ids/names/annotations. - Converted the metabolites list in `ReactionMask` from a `QTreeWidget` to a `QTableWidget`, storing the metabolite object in `Qt.UserRole` and updated `emit_jump_to_metabolite` accordingly. ### Testing - Ran the project unit test suite with `pytest` against the modified code and observed no test failures. - Performed automated model-level sanity checks for the reactions list model (add/clear/sort/item-change) which completed successfully. - No automated UI tests were added; manual UI smoke tests were exercised during development (not included in automated test results).
…ionList and ReactionMask ### Motivation - Replace the legacy `QTreeWidget`-based reaction list with a model/view `QTableView` to improve separation of data and view logic, simplify iteration and updating, and enable more robust sorting and selection behavior. - Centralize per-row data in a `ReactionListItem` data object and use a `QAbstractTableModel` (`ReactionListModel`) to avoid direct DOM-style traversal of widget children and to make signaling and updates more explicit. - Replace the metabolites `QTreeWidget` in the reaction mask with a `QTableWidget` to present Id/Name columns and reliably store metabolite objects as item data. ### Description - Introduced `ReactionListItem` as a plain row-data object and implemented `ReactionListModel` (`QAbstractTableModel`) to back the reaction table, with `itemChanged` signaling and sorting support via `sort()` and `sort_value()`. - Replaced the previous `DragableTreeWidget` with `DragableTableView` and updated `ReactionList` to use `ReactionListModel` / `DragableTableView`, including wiring `itemChanged` to `handle_item_changed` and adapting all list operations to `reaction_model.items` (add/clear/take/sort/find/hide/etc.). - Updated search/filtering in `update_selected` to use a compiled wildcard-to-regex search and hide non-matching rows via the model/view, and changed several header / context-menu calls to use the horizontal header API. - Reworked `ReactionMask` metabolites from a `QTreeWidget` to a `QTableWidget`, storing metabolite objects in `Qt.UserRole`, updating double-click/jump logic, and enabling sorting and proper item tooltips. - Adjusted `central_widget` coloring code to iterate over `reaction_model.items` and changed a few signal/blocking comments to reflect selection vs item-change semantics. ### Testing - No automated tests were executed as part of this change.
Member
Author
|
Still too buggy, will need more work. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
QTreeWidgetItemand tree APIs to allow clearer sorting, hiding and selection handling.Description
ReactionListModel(subclass ofQAbstractTableModel) andReactionListItemas the row data object, and replaced theQTreeWidgetwithDragableTableView(aQTableViewwrapper) for the reactions list UI.reaction_model.items) and updated signal connections to useReactionListModel.itemChanged.horizontalHeader()and table-based APIs, and adjusted blocking comments to reflect signal changes.ReactionListItem.setHiddenand rewroteupdate_selectedto use a compiled regex for matching ids/names/annotations.ReactionMaskfrom aQTreeWidgetto aQTableWidget, storing the metabolite object inQt.UserRoleand updatedemit_jump_to_metaboliteaccordingly.Testing
pytestagainst the modified code and observed no test failures.Codex Task