Apparently QAbstractTableModel.dataChanged() signal has no effect

Question

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:

• [ADD]: add the item selected in the combo above to the list (num = 1).
• [PLUS]: increase num by one in selected row, if any.
• [MINUS]: decrease 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.

Answer 1

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:

1. _index_of_row compares the row contents with the enumeration index (instead that of the items, the dictionaries);
2. even if the comparison were valid, it returns the "found" dictionary instead of the row index;
3. you are changing the value of the row before comparison, thus making the match of _index_of_row invalid;
4. 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.