I am writing a rather large script which, among other things has a table I need to manipulate adding and deleting rows and changing some values.
Underlying data structure (forced on me, I cannot change it) is a rather complex XML where items displayed in table rows are a fixed number and one field (num > 0) indicates if the whole item is valid and thus should be displayed, so I'm using a QSortFilterProxyModel to filter the rows.
Here follows a trimmed down version of the script (it's still too large to be a "minimal example", but I hope it's manageable; I did not want to remove too much f the actual program structure):
from collections import namedtuple
from typing import Optional
from PyQt6.QtCore import QAbstractTableModel, Qt, pyqtSlot, QModelIndex, QSortFilterProxyModel, QObject, pyqtProperty, \
pyqtSignal
from PyQt6.QtWidgets import *
class AbstractModel(QAbstractTableModel):
_column = namedtuple('_column', "name func hint align")
def __init__(self, columns: [_column]):
self._columns = columns
super().__init__()
self._rows = []
# self.select()
def data(self, index, role=...):
match role:
case Qt.ItemDataRole.DisplayRole:
row = None
try:
row = self._rows[index.row()]
return self._columns[index.column()].func(row)
except KeyError:
print(f'ERROR: unknown item in row {row}')
return '*** UNKNOWN ***'
case Qt.ItemDataRole.TextAlignmentRole:
return self._columns[index.column()].align
return None
def headerData(self, section, orientation, role=...):
if orientation == Qt.Orientation.Horizontal:
match role:
case Qt.ItemDataRole.DisplayRole:
return self._columns[section].name
return None
def rowCount(self, parent=...):
return len(self._rows)
def columnCount(self, parent=...):
return len(self._columns)
def select(self):
self.beginResetModel()
self._rows = []
self.endResetModel()
def set_hints(self, view: QTableView):
header = view.horizontalHeader()
for i, x in enumerate(self._columns):
header.setSectionResizeMode(i, x.hint)
def row(self, idx: int):
return self._rows[idx]
all_by_id = [
{'Name': 'foo', 'Type': 'red', 'Level': 0},
{'Name': 'fee', 'Type': 'red', 'Level': 0},
{'Name': 'fie', 'Type': 'red', 'Level': 0},
{'Name': 'fos', 'Type': 'green', 'Level': 0},
{'Name': 'fum', 'Type': 'blue', 'Level': 0},
{'Name': 'fut', 'Type': 'blue', 'Level': 0},
{'Name': 'fam', 'Type': 'yellow', 'Level': 0},
{'Name': 'fol', 'Type': 'yellow', 'Level': 0},
{'Name': 'fit', 'Type': 'magente', 'Level': 0},
]
type_by_id = ['red', 'green', 'blue', 'yellow', 'magenta']
class InventoryModel(AbstractModel):
def __init__(self):
super().__init__([
AbstractModel._column('ID', self.get_id,
QHeaderView.ResizeMode.ResizeToContents,
Qt.AlignmentFlag.AlignRight | Qt.AlignmentFlag.AlignVCenter),
AbstractModel._column('Item', self.get_item,
QHeaderView.ResizeMode.ResizeToContents,
Qt.AlignmentFlag.AlignLeft | Qt.AlignmentFlag.AlignVCenter),
AbstractModel._column('Type', self.get_type,
QHeaderView.ResizeMode.ResizeToContents,
Qt.AlignmentFlag.AlignLeft | Qt.AlignmentFlag.AlignVCenter),
AbstractModel._column('Count', self.get_count,
QHeaderView.ResizeMode.ResizeToContents,
Qt.AlignmentFlag.AlignRight | Qt.AlignmentFlag.AlignVCenter),
])
self._inventory = None
def select(self, what=None):
if self._inventory:
self._inventory.changed.disconnect(self.changed)
self._inventory.rowchanged.disconnect(self.rowchanged)
self._inventory.rowinserted.disconnect(self.rowinserted)
self._inventory = what
self._inventory.changed.connect(self.changed)
self._inventory.rowchanged.connect(self.rowchanged)
self._inventory.rowinserted.connect(self.rowinserted)
self.changed()
def get_id(self, x):
return str(PW.row_item(x))
def get_item(self, x):
return all_by_id[PW.row_item(x)]['Name']
def get_type(self, x):
return all_by_id[PW.row_item(x)]['Type']
def get_count(self, x):
return str(PW.row_num(x))
@pyqtSlot()
def changed(self):
self.beginResetModel()
self._rows = self._inventory.rows
self.endResetModel()
@pyqtSlot(int)
def rowchanged(self, index):
self.dataChanged.emit(self.index(index, 0), self.index(index, len(self._columns)))
@pyqtSlot(int)
def rowinserted(self, index):
self.beginInsertRows(QModelIndex(), index, index)
self._rows = self._inventory.rows
self.endInsertRows()
class InventoryProxy(QSortFilterProxyModel):
def filterAcceptsRow(self, source_row, source_parent):
row = self.sourceModel().row(source_row)
return PW.row_valid(row)
class PW(QObject):
changed = pyqtSignal()
rowchanged = pyqtSignal(int)
rowinserted = pyqtSignal(int)
rowdeleted = pyqtSignal(int)
def __init__(self, parent=None):
super().__init__(parent)
self._store = [
{'num': 1, 'item': 0, 'level': 0},
{'num': 0},
{'num': 0},
{'num': 0},
{'num': 0},
{'num': 0},
{'num': 0},
{'num': 0},
{'num': 0},
{'num': 0},
{'num': 0},
{'num': 0},
]
def _index_of_row(self, row):
for r, n in enumerate(self._store):
if r == row:
return n
return None
@pyqtProperty(int)
def rows(self):
return self._store
@staticmethod
def row_num(row):
return row['num']
@staticmethod
def row_valid(row):
return PW.row_num(row) > 0
@staticmethod
def row_item(row):
return row['item']
@staticmethod
def row_level(row):
return row['level']
def row_inc(self, row, inc):
num = self.row_num(row)
n = num + inc
if n > 0:
row['num'] = n
self.rowchanged.emit(self._index_of_row(row))
else:
row['num'] = 0
self.rowdeleted.emit(self._index_of_row(row))
def add(self, idx):
# FIXME: should check if similar row exists
for n, row in enumerate(self._store):
if self.row_num(row) == 0:
row['num'] = 1
row['item'] = idx
row['level'] = 0
self.rowinserted.emit(n) # FIXME: this removes selection
break
if __name__ == '__main__':
class MainWindow(QMainWindow):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.setWindowTitle('Storage')
self.l1 = QVBoxLayout()
self.w1 = QWidget()
self.w1.setLayout(self.l1)
self.cb = QComboBox(self)
self.cb.addItems([x['Name'] for x in all_by_id])
self.l1.addWidget(self.cb)
self.w2 = QWidget()
self.l2 = QHBoxLayout()
self.w2.setLayout(self.l2)
self.ba = QPushButton('ADD')
self.l2.addWidget(self.ba)
self.bp = QPushButton('PLUS')
self.l2.addWidget(self.bp)
self.bm = QPushButton('MINUS')
self.l2.addWidget(self.bm)
self.l1.addWidget(self.w2)
self.storage = QTableView()
self.storage.setSelectionBehavior(QAbstractItemView.SelectionBehavior.SelectRows)
self.l1.addWidget(self.storage)
self.setCentralWidget(self.w1)
self.storage_wrapper: Optional[PW] = None
self.storage_model: Optional[InventoryModel] = None
self.storage_proxy: Optional[InventoryProxy] = None
self.ba.clicked.connect(self.add)
self.bp.clicked.connect(self.plus)
self.bm.clicked.connect(self.minus)
def set_storage_model(self, wrapper: PW):
self.storage_wrapper = wrapper
self.storage_model = InventoryModel()
self.storage_proxy = InventoryProxy()
self.storage_model.select(self.storage_wrapper)
self.storage_proxy.setSourceModel(self.storage_model)
self.storage.setModel(self.storage_proxy)
self.storage_model.set_hints(self.storage)
self.storage.setSortingEnabled(True)
self.storage.sortByColumn(2, Qt.SortOrder.AscendingOrder)
@pyqtSlot()
def add(self):
n = self.cb.currentIndex()
self.storage_wrapper.add(n)
@pyqtSlot()
def plus(self):
sel = self.storage.currentIndex()
if sel.isValid():
orig = self.storage_proxy.mapToSource(sel)
row = self.storage_model.row(orig.row())
self.storage_wrapper.row_inc(row, 1)
@pyqtSlot()
def minus(self):
sel = self.storage.currentIndex()
if sel.isValid():
orig = self.storage_proxy.mapToSource(sel)
row = self.storage_model.row(orig.row())
self.storage_wrapper.row_inc(row, -1)
@pyqtSlot()
def add(self):
n = self.cb.currentIndex()
self.storage_wrapper.add(n)
app = QApplication([])
win = MainWindow()
pw = PW()
win.set_storage_model(pw)
win.show()
from sys import exit
exit(app.exec())
Meaning of the three buttons is:
num = 1).num by one in selected row, if any.num by one in selected row, if any; if num goes to 0` the whole row should disappear.This works... almost.
Main problem is [PLUS] and [MINUS] action is not immediately reflected in the table (I need to force refresh by switching focus) in spite of (apparently correct)
self.dataChanged.emit(self.index(index, 0), self.index(index, len(self._columns)))
Also row deletion doesn't work correctly (QSortFilterProxyModel.filterAcceptsRow() doesn't seem to be called again), so I'm missing some other bit somewhere.
When selections are not correctly kept (including when sorting/filtering is active) or data is not visually updated, it almost always means that somewhere a wrong QModelIndex is provided: item views have automatic mechanisms that work around small model inconsistencies (preventing fatal errors) but these issues are clear symptoms of invalid indexes.
Your problem is actually caused by four mistakes:
_index_of_row compares the row contents with the enumeration index (instead that of the items, the dictionaries);_index_of_row invalid;dataChanged() uses an invalid bottomRight index as it is based on a non existent column (len() instead of len() - 1);Here are the required changes:
class InventoryModel(AbstractModel):
def rowchanged(self, row):
self.dataChanged.emit(
self.index(row, 0),
self.index(row, len(self._columns) - 1)
)
...
class PW(QObject):
def _index_of_row(self, data):
for row, other in enumerate(self._store):
if other == data:
return row
return -1
...
def row_inc(self, row, inc):
# first, look up for the row number
rowNumber = self._index_of_row(row)
if rowNumber < 0:
# no matching row?
return
# then, update the num value
num = self.row_num(row)
row['num'] = max(0, num + inc)
# emit the signal with the original row number
self.rowchanged.emit(rowNumber)
I am not completely sure about the removal of the rowdeleted signal in row_inc above, as your code is quite complex, and you actually never connected that signal with anything, by the way; sending the rowchanged signal no matter the value seems to fix the item removal in the proxy, though.
An important suggestion, if I may: consider very carefully to improve your naming choices, as having short and ambiguous names will make debugging more painful than it already is.
For instance, your usage of row and index is confusing, as you use both of them to indicate the row index, and the former to also indicate the row content.
Your usage of extremely short names won't help too. It may be acceptable in small contexts (like for loops in short functions), but you have to ensure that those names (or letters) are clearly recognizable and explanatory, otherwise mistakes like your confusion about r and n in _index_of_row will always be around the corner. row_inc is another similar example: num and n are quite indistinguishable, and even if they refer to the same context, it's important to make it clear what's their purpose within the code.
Extremely short names have very few benefits (less typing, shorter code lines), but lots of drawbacks and no performance improvement at all. On the contrary: what you would gain from them is almost nothing if compared to the time you spend to contextualize them every time you read them, especially when debugging (possibly after hours of coding), not to mention when other people has to read and understand your code.