unwanted empty rows in a proxy model

Question

I have a question regarding Qt's model/view architecture.

I have implemented a class TestModel inheriting from QAbstractItemModel, with a custom method TestModel.addRevision(...) to insert new rows and TestModel.removeRevision(...) to remove rows. My model has a hierarchical structure, and I have different methods add_X and remove_X for different levels in the tree.

Of course, as per the documentation, I call the required functions before inserting or removing rows like this (the ellipsis handles the retrieval of information based on my data source, is lengthy and I believe is not necessary to show)

def add_revision(self, name:str):
    parent_index = ...
    new_row_in_parent = ...
    self.beginInsertRows(parent_index, new_row_in_parent, new_row_in_parent)
    ...
    self.endInsertRows()

I am inserting the rows one by one, and notice I do not the very common mistake of adding one too many rows by calling self.beginInsertRows(parent, start, end) with end = start +1.

The methods for removal have quite the similar structure.

I can veryify that my model works fine by attaching a QTreeView to it. Now I also have an update method, which does the following (in pseudo-code):

# models.py
class TestModel(QtCore.QAbstractItemMOdel):
    ...
    def __init__(self, parent=None):
        ...
        self.update()
    def update(self):
        # remove all items one by one using remove_revision in a foreach loop
        # scan the source for new (updated) revisions
        # add all revisions found one by one in a foreach loop

On the model, this function also works as intended, and once I trigger the update, the view automatically updates, too. Notice I am using update as well during the initialization.

Next step, I implemented a proxy model for sorting and filtering. My problem is even reproducible with a default QSortFilterProxyModel, without setting any filter.

I set the view up like this:

...
view = QTreeView(self)
model = TestModel(self)
proxy_model = QSortFilterModel(self)
proxy_model.setSourceModel(model)
view.setModel(proxy_model)

Right after initialization, the view display as intended (see screenshot below)

Then after I trigger update, the view display changes to

where these nasty empty rows are added. They are not selectable, unlike the "good" rows, and I can't figure out where they are coming from. I tried replacing the QSortFilterProxyModel with a QIdentityProxyModel and the extra rows went away, so I am very confident that the empty rows are added only in the QSortFilterProxModel. However, this is the default implementation, I did not yet override any of the sort and filter methods.

Interestingly, when I use the QIdentityProxyModel, the view shows all items in a collapsed state after calling update, while with the QSortFilterProxyModel the items stay expanded.

Question:

It seems it is not enough to call beginInserRows and endInsertRows. Do I need to emit other signals to inform the proxy model of updates?

Or, does the proxy model update too fast before all the removal is done in the source model?

Edit 1

as per request, this is the full update method of my model. I also included other classes and methods in use:

updating the model:

def update(self, skip: bool = True):

    revisions_to_remove = []
    files_to_inspect = []

    for (index, key) in enumerate(self.lookup_virtual_paths):
        #  first remove everything beneath file
        obj = self.lookup_virtual_paths.get(key, None)

        if obj is None:
            continue

        if isinstance(obj, Revision):
            revisions_to_remove.append(obj)

        if isinstance(obj, File):
            files_to_inspect.append(obj)

    #  first remove revisions
    for revision in revisions_to_remove:
        self.remove_revision(revision)
        pass

    file: File
    for file in files_to_inspect:
        # add revisions
        # construct the filesystem path to lookup
        scraper: ScraperVFSObject = file.parent().parent()
        if scraper is None:
            log.warning('tbd')
            return

        path_scraper = Path(scraper.fs_storage_path())
        if not path_scraper.exists():
            w = 'path does not exist "%s"' % (
                path_scraper.absolute().as_posix())
            log.warning(w, path_scraper)
            return

        path_file = path_scraper / Path(file.machine_name)
        if not path_file.exists():
            w = 'path does not exist "%s"' % (
                path_file.absolute().as_posix())
            log.warning(w)
            return

        for elem in path_file.glob('*'):
            if not elem.is_dir():
                continue

            if not len(elem.name) == len(ScraperModel.to_timeformat(datetime.now())):
                continue

            actual_file = elem / \
                Path('%s_%s.html' % (file.machine_name, elem.name))
            if not actual_file.exists():
                continue

            self.add_revision(
                ScraperModel.from_timeformat(elem.name),
                actual_file.absolute().as_posix(),
                file.virtual_path())

adding of revisions:

def add_revision(self, dt: datetime, file: str, to: str, skip=False):
    f = self.lookup_virtual_paths.get(to, None)
    if f is None:
        w = 'trying to add revision "%s" to virtual path "%s"' % (dt, to)
        log.warning(w)
        return

    r = Revision(dt, file, f, self)

    parent_index = r.parent().get_model_index()
    start = r.get_row_in_parent()

    self.beginInsertRows(parent_index, start, start)

    self.add_to_lookup(r)

    # announce that revision has been added
    self.endInsertRows()


    #  immediately add thumbnail groups to the revision,
    #  because a thumbnail-group can only exist in the revision
    known = ThumbnailGroupKnown(r, self)
    unknown = ThumbnailGroupUnknown(r, self)
    ignored = ThumbnailGroupIgnored(r, self)

    start = known.get_row_in_parent()
    end = ignored.get_row_in_parent()
    self.beginInsertRows(r.get_model_index(), start, end)
    self.add_to_lookup([known, unknown, ignored])
    self.endInsertRows()

removing revisions:

def remove_revision(self, revision: "Revision"):
    #  first get ModelIndex for the revision
    parent_index = revision.parent().get_model_index()
    start = revision.get_row_in_parent()

    #  first remove all thumbnail groups
    tgs_to_remove = []
    for tg in revision.children():
        tgs_to_remove.append(tg)

    tg: ThumbnailGroup
    for tg in tgs_to_remove:
        self.beginRemoveRows(tg.parent().get_model_index(),
                             tg.get_row_in_parent(),
                             tg.get_row_in_parent())
        vpath = tg.virtual_path()
        tg.setParent(None)
        del self.lookup_virtual_paths[vpath]
        self.endRemoveRows()


    self.beginRemoveRows(parent_index, start, start)
    key = revision.virtual_path()

    # delete the revision from its parent
    revision.setParent(None)

    #  delete the lookup
    del self.lookup_virtual_paths[key]
    self.endRemoveRows()

Edit 2

As per suggestion of @Carlton, I reordered statements in remove_revision. I understand, this could have easily been a problem (now or later). The implementation now reads:

def remove_revision(self, revision: "Revision"):
    #  first remove all thumbnail groups
    tgs_to_remove = []
    for tg in revision.children():
        tgs_to_remove.append(tg)

    tg: ThumbnailGroup
    for tg in tgs_to_remove:
        self.beginRemoveRows(tg.parent().get_model_index(),
                             tg.get_row_in_parent(),
                             tg.get_row_in_parent())
        vpath = tg.virtual_path()
        tg.setParent(None)
        del self.lookup_virtual_paths[vpath]
        self.endRemoveRows()


    parent_index = revision.parent().get_model_index()
    start = revision.get_row_in_parent()
    self.beginRemoveRows(parent_index, start, start)
    key = revision.virtual_path()

    # delete the revision from its parent
    revision.setParent(None)

    #  delete the lookup
    del self.lookup_virtual_paths[key]
    self.endRemoveRows()

I later plan to pass the index directly, but for debugging I decided to temporaily store it. However, ther problematic behaviour still exists unchanged.

Edit 3

So as per suggestion by @Carlton, the "phantom rows" seem to be a problem of a rowCount mismatch with the actual data.

I rearranged some more code in the add_revision method to have the following for thumbnail groups:

 def add_revision(self, dt: datetime, file: str, to: str, skip=False):
    ...
    # no changes before here

    print('before (add_revision)', len(r.children()),
          self.rowCount(r.get_model_index()))

    self.beginInsertRows(r.get_model_index(), 0, 2)
    known = ThumbnailGroupKnown(r, self)
    unknown = ThumbnailGroupUnknown(r, self)
    ignored = ThumbnailGroupIgnored(r, self)
    self.add_to_lookup([known, unknown, ignored])
    self.endInsertRows()

    print('after (add_revision)', len(r.children()),
          self.rowCount(r.get_model_index()))

As you can see, I manually picked the start and end arguments. With this modification, I can put the data insertion actually in between beginInsertRows and endInsertRows and the "phantom rows" dissapear. However, then I have a new problem: I can't usually know beforehand, at which indices the new rows will appear. This seems like a good use for the suggested layoutAboutToBeChanged signal, but how can I, in pyside6, pass the parent list?

Edit 4: MWE

Here is a minimal working example. You need to have PySide6 installed. MWE And the same code to be hosted here directly:

import sys
from PySide6 import (
    QtCore,
    QtWidgets
)


class Node(QtCore.QObject):
    def __init__(self, val: str, model, parent=None):
        super().__init__(parent)
        self.value = val
        self._model = model

    def child_count(self) -> int:
        return len(self.children())

    def get_child(self, row: int) -> "Node":
        if row < 0 or row >= self.child_count():
            return None
        else:
            return self.children()[row]

    def get_model_index(self) -> QtCore.QModelIndex:
        return self._model.index(self.get_row_in_parent(), 0, self.parent().get_model_index())

    def get_row_in_parent(self) -> int:
        p = self.parent()
        if p is not None:
            return p.children().index(self)

        return -1


class RootNode(Node):
    def get_row_in_parent(self) -> int:
        return -1

    def get_model_index(self) -> QtCore.QModelIndex:
        return QtCore.QModelIndex()


class Model(QtCore.QAbstractItemModel):
    def __init__(self, parent=None):
        super().__init__(parent)

        self.root_item = None

        # simulate the changing data
        self._data = [
            (1, 'child 1 of 1'),
            (1, 'child 2 of 1'),
            (1, 'child 3 of 1'),
        ]

        self._initialize_static_part()

        self.update()

    def _initialize_static_part(self):
        """This is the part of my model which never changes at runtime
        """
        self.root_item = RootNode('root', self, self)

        nodes_to_add = []

        for i in range(0, 5):
            new_node = Node(str(i), self)
            nodes_to_add.append(new_node)

        for node in nodes_to_add:
            self.add_node(node, self.root_item)

    def update(self):
        """This is the part which needs update during runtime
        """

        rows_to_add = []
        rows_to_delete = []

        self.layoutAboutToBeChanged.emit()

        for c in self.root_item.children():
            for d in c.children():
                rows_to_delete.append(d)

        for (parent_identifier, name) in self._data:
            node = Node(name, self)
            #  actually, the future parent is a different function, but for the MWE this suffices
            future_parent = self.root_item.get_child(parent_identifier)
            rows_to_add.append((future_parent, node))

        for node in rows_to_delete:
            self.remove_node(node)

        for (parent, node) in rows_to_add:
            self.add_node(node, parent)

        self.layoutChanged.emit()

    def add_node(self, node: Node, parent: Node):
        self.beginInsertRows(parent.get_model_index(),
                            parent.child_count(),
                            parent.child_count())
        node.setParent(parent)
        self.endInsertRows()

    def remove_node(self, node):
        parent_node = node.parent()
        row = parent_node.get_model_index().row()
        self.beginRemoveRows(parent_node.get_model_index(
        ), row, row)
        # print(parent_node.get_model_index().isValid())

        node.setParent(None)

        # print(node)
        # print(parent_node.children())

        self.endRemoveRows()
        # reimplement virtual method

    def columnCount(self, parent: QtCore.QModelIndex = QtCore.QModelIndex()) -> int:
        return 1

    # reimplement virtual method
    def rowCount(self, parent: QtCore.QModelIndex = QtCore.QModelIndex()) -> int:
        if not parent.isValid():
            parent_item = self.root_item
        else:
            parent_item = parent.internalPointer()

        return parent_item.child_count()

    # reimplement virtual method
    def index(self, row: int, column: int, parent: QtCore.QModelIndex = QtCore.QModelIndex()) -> QtCore.QModelIndex:
        if not self.hasIndex(row, column, parent):
            return QtCore.QModelIndex()

        if not parent.isValid():
            parent_item = self.root_item
        else:
            parent_item = parent.internalPointer()

        child_item: Node = parent_item.get_child(row)
        if child_item is not None:
            return self.createIndex(row, column, child_item)

        return QtCore.QModelIndex()

    # reimplement virtual method
    def parent(self, index: QtCore.QModelIndex) -> QtCore.QModelIndex:
        if not index.isValid():
            return QtCore.QModelIndex()

        child_item: Node = index.internalPointer()
        parent_item = child_item.parent()

        if parent_item is not None:
            return parent_item.get_model_index()

        return QtCore.QModelIndex()

    # reimplement virtual method
    def data(self, index: QtCore.QModelIndex, role: int = QtCore.Qt.DisplayRole) -> object:
        if not index.isValid():
            return None

        if role == QtCore.Qt.DisplayRole:
            item: Node = index.internalPointer()
            if item is not None:
                return item.value
            return 'whats this?'

        return None


class MyWindow(QtWidgets.QMainWindow):
    defaultsize = QtCore.QSize(780, 560)

    def __init__(self, app, parent=None):
        super().__init__(parent)
        self.app = app
        self.resize(self.defaultsize)
        main_layout = QtWidgets.QSplitter(QtCore.Qt.Vertical)
        self.panel = Panel(main_layout)
        self.setCentralWidget(main_layout)

        self.model = Model(self)

        proxy_model1 = QtCore.QSortFilterProxyModel(self)
        proxy_model1.setSourceModel(self.model)

        proxy_model2 = QtCore.QIdentityProxyModel(self)
        proxy_model2.setSourceModel(self.model)

        view1 = QtWidgets.QTreeView(self.panel)
        view1.setAlternatingRowColors(True)
        view1.setModel(proxy_model1)
        view1.expandAll()

        view2 = QtWidgets.QTreeView(self.panel)
        view2.setAlternatingRowColors(True)
        view2.setModel(proxy_model2)
        view2.expandAll()

        self.panel.addWidget(view1)
        self.panel.addWidget(view2)

        # we simulate a change, which would usually be triggered manually
        def manual_change_1():
            self.model._data = [
                (1, 'child 2 of 1'),
                (1, 'child 3 of 1'),
            ]
            self.model.update()

        QtCore.QTimer.singleShot(2000, manual_change_1)


class App(QtWidgets.QApplication):
    def __init__(self):
        super().__init__()
        self.window = MyWindow(self)

    def run(self):
        self.window.show()
        result = self.exec_()
        self.exit()


class Panel(QtWidgets.QSplitter):
    pass


if __name__ == '__main__':
    app = App()
    app.startTimer(1000)

    sys.exit(app.run())

Answer 1

While a little late, I was experiencing this same issue on Qt 5.X and noticed that I needed to manually hook up an invalidate call to the row insertion/removal signals.

// cpp
connect(base, &SomeBaseModel::rowsInserted, proxy, &MyProxyModel::invalidate);
connect(base, &SomeBaseModel::rowsRemoved, proxy, &MyProxyModel::invalidate);

# python
base.rowsInserted.connect(proxy.invalidate)
base.rowsRemoved.connect(proxy.invalidate)

After that, the proxy and base aligned correctly. I'm assuming the same could be applied for the ::rowsMoved signal but I didn't need this in my specific case.

I choose this over a layoutChanged approach to avoid over-compute on large, complex model data.