[FIXED] Python/PySide6: Crashes after Click sequence between treeView and tableView

Issue

I experience crashes after certain conditions are met in my current project. It took me some time to really pinpoint the problem.

I'm using Python 3.11 with PySide6. In a nutshell, the app shows:

• treeView (with QStandardItemModel)
• tableView1 (with QAbstractTableModel)
• tableView2 (with QAbstractTableModel and QSortFilterProxyModel)

Function-wise, everything works as intended. Basic funtionality:

• User chooses item from treeView -> tableView1 shows all attributes inside the chosen object -> tableView2 shows contents of a SQL database for all entries matching the chosen object.
• User can now interact with tableView1 + tableView2 (clicking, editing...)

Problem (click sequence):

1. Click on item in treeView -> tableView1 and 2 are updating
2. Click on column header in tableView2 (.setSortingEnabled(True))
3. Click on cell in tableView2 (just highlighting)
4. Click on different item in treeView -> tableView1 and 2 update
5. Click on column header in tableView2 again -> crash (mostly on the first click, but sometimes you can click a few times before it crashes)

Only this sequence leads to the crash. If you miss a step, it won't crash. With "crash" I mean:

• no exception is raised, the thread just closes
• debugger doesn't stop @exception, it just crashes

faulthandler stack trace doesn't show too much info...

IMPORTANT: I already solved a very similar bug: Click sequence:

1. Click on item in treeView -> tableView1 and 2 are updating
2. Click on cell in tableView2 (just highlighting)
3. Click on different item in treeView -> crash (mostly on the first click, but sometimes you can click a few times before it crashes)

I noticed, after clicking the cell in tV2, if I click somewhere else (for example a cell in tV1), it then won't crash when clicking treeView again. So my assumption was, there has to be something stored somewhere, when the cell in tV2 is clicked. After a quick research I tried deleting the selection data of tV2 before treeView clicking action is carried out:

selection_model_tV2 = self.ui.tabView_2.selectionModel()
selection_model_tV2.clear()

That worked!

So naturally my first approach (for the current bug) was to also clear the selection data of treeView, tV1, tV2 and a mix of those, but nothing worked out so far...

For the current one, I'm not sure which .selectionModel() is responsible (if any) for the bug, therefore I tried clearing treeView, tV1, tV2 and combinations.

Additional info:

• If I disable QSortFilterProxy, both crashes wouldn't happen...

UPDATE: In the following, a reproducible example:

import faulthandler
import sys

import pandas as pd
from PySide6.QtCore import QAbstractTableModel, QSortFilterProxyModel, Qt
from PySide6.QtGui import QStandardItem, QStandardItemModel
from PySide6.QtWidgets import (
QApplication,
QHBoxLayout,
QHeaderView,
QMainWindow,
QTableView,
QTreeView,
QWidget,)

faulthandler.enable()


class CustomTableModel(QAbstractTableModel):
    def __init__(self, table_data=pd.DataFrame(), parent=None):
        super(CustomTableModel, self).__init__()
        self.table_data = table_data
        self.parent = parent

    def data(self, index, role):
        value = self.table_data.iat[index.row(), index.column()]
        if role == Qt.DisplayRole or role == Qt.EditRole:
            return value

    def setData(self, index, value, role):
        return False

    def rowCount(self, index):
        return self.table_data.shape[0]

    def columnCount(self, index):
        return self.table_data.shape[1]


class CustomHeaderView(QHeaderView):
    def __init__(self, orientation, parent=None):
        super(CustomHeaderView, self).__init__(orientation, parent)
        self.setSectionsClickable(True)

    def mousePressEvent(self, event):
        super().mousePressEvent(event)


class CustomProxyModel(QSortFilterProxyModel):
    def __init__(self, parent=None):
        super(CustomProxyModel, self).__init__(parent)

    def sort(self, column, order):
        return super().sort(column, order)


class SimpleGUI(QMainWindow):
    def __init__(self):
        super().__init__()

        self.setWindowTitle("Simple PySide6 GUI")
        self.setGeometry(100, 100, 800, 600)

        # Create a QTreeView on the left
        self.tree_view = QTreeView()
        self.tree_view.setHeaderHidden(True)
        self.tree_view.setRootIsDecorated(True)

        # Create a QFileSystemModel to populate the QTreeView
        self.treeView_model = QStandardItemModel(self)
        self.tree_view.setModel(self.treeView_model)
        self.init_treeView()
        self.tree_view.clicked.connect(self.treeView_clicked)

        # Create a QTableView on the right
        self.table_view = QTableView()
        self.tableView_model = CustomTableModel(parent=self)

        # COMMENT THESE LINES FOR QSORTFILTERPROXY BYPASS
        self.proxy_model = CustomProxyModel(self)
        self.proxy_model.setSourceModel(self.tableView_model)
        self.table_view.setModel(self.proxy_model)

        # UNCOMMMENT FOR QSORTFILTERPROXY BYPASS
        # self.table_view.setModel(self.tableView_model)

        self.header_view = CustomHeaderView(Qt.Orientation.Horizontal, self)
        self.table_view.setHorizontalHeader(self.header_view)
        self.table_view.setSortingEnabled(True)

        self.table_view_data_dict = {
            "item1": ["1_1", "1_2", "1_3"],
            "item2": ["2_1", "2_2", "2_3"],
            "item3": ["3_1", "3_2", "3_3"],
        }

        # Create a vertical layout for the main window
        main_layout = QHBoxLayout()

        # Add the treeView to the layout
        main_layout.addWidget(self.tree_view)

        # Add the tableView to the layout
        main_layout.addWidget(self.table_view)

        # Create a central widget and set the main layout
        central_widget = QWidget()
        central_widget.setLayout(main_layout)

        # Set the central widget for the main window
        self.setCentralWidget(central_widget)

    def init_treeView(self):
        parent = self.treeView_model.invisibleRootItem()
        data = [
            "item1",
            "item2",
            "item3",
        ]
        for element in data:
            child = QStandardItem(element)
            parent.appendRow(child)

    def treeView_clicked(self, index):
        self.clicked_treeView_item = self.treeView_model.itemFromIndex(index).text()
        df_for_tableView = pd.DataFrame(self.table_view_data_dict[self.clicked_treeView_item]).T
        self.tableView_model.table_data = df_for_tableView
        self.tableView_model.layoutChanged.emit()


if __name__ == "__main__":
    app = QApplication(sys.argv)
    window = SimpleGUI()
    window.show()
    sys.exit(app.exec_())

Both bugs are reproducible:

1. click on item in treeView, click on cell in tableView, click on treeView again -> crash (you'll have to try a few times)
2. click on treeView, click on column header, click on cell, click on treeView, click on column header again -> crash (you'll have to try a few times)

Any suggestions?

Thank you in advance

Solution

There are some tutorials (and posts here on SO) that implicitly suggest that when the content of the table changes it's enough to emit the layoutChanged signal.

Unfortunately, this is almost always wrong, or, at least, insufficient.

While only emitting layoutChanged often works, it's just because Qt item views are implemented in ways that are smart enough to normally ignore some missing aspects, but, in reality, we should never rely on this.

Most QAbstractItemModels have signals that must be emitted in pairs and in the correct order whenever the structure of the model changes: shape (row/column count, including those of children for tree models) and layout (item order).

The concept is always based on three steps, in this precise order:

1. emit the "about to do X" signal;
2. change the underlying data;
3. emit the "X completed" signal;

Properly following the above procedure ensures that all correct signals are sent in the right order to the connected views, which internally keep references to persistent indexes and eventually remap them when the change has completed.

Failing to do that normally results in having inconsistent selections for the best case scenario, but can also cause drawing/update issues and, like in your case, fatal crashes.

For instance, when adding a row, you must first emit rowsAboutToBeInserted (even if it's just one row), then insert the row in the internal data, and finally call rowsInserted. The normal procedure is to use the existing functions: beginInsertRows() and endInsertRows(), which also internally emit the related signals.

If the layout changes, this is achieved with layoutAboutToBeChanged and layoutChanged signals (again, both signals must be emitted in this order).

Interestingly enough, the layoutChanged signal documentation points out an important requirement that is exactly the cause of your problem.:

When subclassing QAbstractItemModel or QAbstractProxyModel, ensure that you emit layoutAboutToBeChanged() before changing the order of items or altering the structure of the data you expose to views, and emit layoutChanged() after changing the layout.

Note that, in this context, "layout" means the change is in the order in which items are placed within the same structure, while the row/column count remains unchanged. For example, when moving rows or columns, or even for sorting.

This is clearly not your case, as you're possibly changing the whole contents of your model (including row/column count, the shape), so you actually need to use the model reset signals: modelAboutToBeReset and modelReset. In reality, similarly to the row insertion above, it's always better to call the dedicated functions: beginResetModel() and endResetModel().

Finally, following proper OOP principles of encapsulation and separation of concerns, you should never call/emit the above functions/signals from outside the model. A better solution is to create a custom function in the model, and do all the above from there.

class CustomTableModel(QAbstractTableModel):
    ...
    def setTableData(self, df):
        self.beginResetModel()
        self.table_data = df
        self.endResetModel()


class SimpleGUI(QMainWindow):
    ...
    def treeView_clicked(self, index):
        key = index.data()
        if key in self.table_view_data_dict:
            table_data = pd.DataFrame(table_view_data_dict[key]).T
            self.tableView_model.setTableData(table_data)

Also note that even if you're actually using your model on a table view, both rowCount() and columnCount() should always have the optional parent argument (defaulting to QModelIndex(), implying the root), because overriding must always be consistent with the signature of the overridden function. For the same reason, both data() and setData() should have a default role.

Coincidentally, this relates to the "smart implementation" explained above, which should not be given as granted.

class CustomTableModel(QAbstractTableModel):
    ...
    # note the "role" default argument
    def data(self, index, role=Qt.DisplayRole):
        value = self.table_data.iat[index.row(), index.column()]
        if role == Qt.DisplayRole or role == Qt.EditRole:
            return value

    # as above, but the default role of setData() is EditRole;
    # note that this override is actually unnecessary, since the
    # default behavior of QAbstractItemModel (which is directly
    # called by default from QAbstractTableModel) does nothing, and
    # already returns False
    def setData(self, index, value, role=Qt.EditRole):
        return False

    def rowCount(self, index, parent=QModelIndex()):
        return self.table_data.shape[0]

    def columnCount(self, index, parent=QModelIndex()):
        return self.table_data.shape[1]

If you're completely sure that your model is mono (a list) or bi dimensional (a table), then the parent can be None.

Answered By - musicamante