8

I'm new to model view and I have been following this tutorial while checking the documentation at the same time and I stumbled upon this little detail : The code of the tutorial which can be downloaded here has in the QAbstractItemModel class (here QAbstractListModel) the setData method which code is :

def setData(self, index, value, role = QtCore.Qt.EditRole):
    if role == QtCore.Qt.EditRole:

        row = index.row()
        color = QtGui.QColor(value)

        if color.isValid():
            self.__colors[row] = color
            self.dataChanged.emit(index, index)
            return True
    return False

According to the explanations in the tutorial and from what I understood from the documentation, if the function returns True, then the view is updated, if it returns false, nothing happens, but when I changed the code to :

def setData(self, index, value, role = QtCore.Qt.EditRole):
    if role == QtCore.Qt.EditRole:

        row = index.row()
        color = QtGui.QColor(value)

        if color.isValid():
            self.__colors[row] = color
            self.dataChanged.emit(index, index)
            return False # This is what I changed in the code
    return False

I realized that the view still gets updated if color.isValid() even if the function returns False. Am I misunderstanding the return role in the setData method or is it a bug ?

For reference, I'm using PySide 1.2.1, not PyQt4.

László Papp
  • 51,870
  • 39
  • 111
  • 135
Ilyes Ferchiou
  • 583
  • 3
  • 10
  • 22

3 Answers3

5

To quote from the video tutorial regarding setData:

...this function needs to return true if the operation was successful, or else the view will not update itself.

Strictly speaking, this statement is false. The documentation for QAbstractItemModel only says that setData returns true if the data was set successfully, and false otherwise; it does not mention what the consequences of this might be. Specifically, it does not mention anything about updating the view.

Looking at the Qt source code, the return value of setData does get checked in a few places, and some of those checks can sometimes help trigger an update. But there are literally dozens of things that can trigger an update, so the return value of setData is in no way essential for updating items.

Perhaps it would have been more accurate to say that setData should return true, otherwise the view may not update itself (under certain circumstances).

ekhumoro
  • 115,249
  • 20
  • 229
  • 336
  • Thank you, this is what I wanted to know. So if not set correctly it might mess up somewhere after all. – Ilyes Ferchiou Dec 27 '13 at 06:54
  • @IlyesFerchiou: this is what I wrote, but in lotta more details. :))) You are not being objective now despite the lot of hand-holding yesterday and effort yeterday. Either way, I am happy that you have a solution now. – László Papp Dec 27 '13 at 07:00
  • @LaszloPapp. The way I see it, your answer and comments dealt well with the issue of how to implement `setdata` correctly, but it didn't directly address the claim made in the video tutorial. Possibly the OP should have made it clearer what he was _really_ asking about. Anyway, I gave you an upvote because your answer and comments do seem to cover all the points in the original question. – ekhumoro Dec 27 '13 at 07:22
  • @ekhumoro: it does address the tutorial code, both in my comments as well as answer. Please read this again: "Also, the tutorial you seem to refer to, is using the self.__colors set in your code for the rowCount(), `data(), and other methods. If you would like to avoid the update, you will need to return False before any such statement." Also, your answer reads (at least to me) like it updates "randomly" which is incorrect, and it does not provide actual details for the sometimes unlike my answer. Anyway, I think the OP just got frustrated that he had to understand so many things. – László Papp Dec 27 '13 at 07:25
  • Anyway, I am happy to forget this thread, and move on by knowing the OP feels happy now, and has the answer he wanted to hear. – László Papp Dec 27 '13 at 07:29
  • @LaszloPapp. I think we are not really in any disagreement. You were looking at it from the Python side, and I was looking at it from the Qt side. Between us, the OP seems to have got what he wanted, so our job is done. – ekhumoro Dec 27 '13 at 07:47
  • @ekhumoro: I was looking from both sides. I even pasted the relevant setData Qt documentation, and explained the Qt API why it is as is. Either way, I agree that job is done, so let us move on. :-) – László Papp Dec 27 '13 at 07:52
  • @FinalContest Coming in after the fact, and finding this question after working with the same tutorial, my impression is this: ekhumoro's answer is pithy, easier to understand, seems more to the point, and (this is nontrivial, if irrelevant) comes off less condescending. – eric Jul 20 '14 at 20:57
  • @neuronet: I am not sure what you are writing about. – László Papp Jul 21 '14 at 00:41
  • @FinalContest just my impression upon quickly reading both answers. Perhaps condescending isn't right word, just maybe a tad prickly. I tried to figure out how to NOT make that a comment (e.g., start a 'discussion' about it rather than a comment thread), because this is really off topic. Sorry. – eric Jul 21 '14 at 01:09
2

Am I misunderstanding the return role in the setData method or is it a bug ?

From the Qt documentation:

bool QAbstractItemModel::setData(const QModelIndex & index, const QVariant & value, int role = Qt::EditRole) [virtual]

Sets the role data for the item at index to value.

Returns true if successful; otherwise returns false.

The dataChanged() signal should be emitted if the data was successfully set.

The base class implementation returns false. This function and data() must be reimplemented for editable models.

Yet, you seem to emit the dataChanged() signal even if the data is not successfully set based on the return value. Also, the tutorial you seem to refer to, is using the self.__colors set in your code for the rowCount(), `data(), and other methods. If you would like to avoid the update, you will need to return False before any such statement.

You need to pay attention to these criterias because the signal and the colors are managed internally while the return value is used by the caller to see if the setData() method had been successfully set.

Based on the knowledge above, you should have written this code for your second attempt to make it work as you expect it to:

def setData(self, index, value, role = QtCore.Qt.EditRole):
    if role == QtCore.Qt.EditRole:

        row = index.row()
        color = QtGui.QColor(value)

        if color.isValid():
            return False
    return False
Community
  • 1
  • 1
László Papp
  • 51,870
  • 39
  • 111
  • 135
  • Thanks, but what I don't understand, if everything is related to the dataChanged signal, why the need to return True or False ? – Ilyes Ferchiou Dec 26 '13 at 12:38
  • @IlyesFerchiou: so that the caller can check if the update had happened successfully. They do not deal with the signals. They are only left with the return value. – László Papp Dec 26 '13 at 12:39
  • Thanks for your answer but I just removed the dataChanged signal line, and the behavior is still the same, modifications are reflected even if return False. I commented too fast I guess. So we are back to the start. – Ilyes Ferchiou Dec 26 '13 at 12:40
  • @IlyesFerchiou: I cannot reproduce your issue. Please make sure you start it cleanly from scratch please with the code I pasted. – László Papp Dec 26 '13 at 12:42
  • The only thing that changed when removing dataChanged is that only the view on which I carried the modification got updated, the other views using the same model did not update until I set focus on them with the mouse. – Ilyes Ferchiou Dec 26 '13 at 12:43
  • @IlyesFerchiou: is it updated if you only have a False return in the setData method? Does it get updated if you return with False right after "if role == QtCore.Qt.EditRole:". I think something is stray. – László Papp Dec 26 '13 at 12:47
  • If I return False right after "if role == QtCore.Qt.EditRole:" or even right after "if color.isValid():" it doesn't update, but then even if I set it to true, it doesn't update either. – Ilyes Ferchiou Dec 26 '13 at 12:53
  • So the error must be in your code `self.__colors[row] = color` which you have not showed yet. Either way, I updated the answer, and based on your description, it works for you as well. If you want more, you will need to paste more code. – László Papp Dec 26 '13 at 12:54
  • As I said, the view does NOT update if I remove self.__colors[row] = color even if I return TRUE. I did not write the code, as I said, I downloaded it from [here](http://www.youtube.com/redirect?q=http%3A%2F%2Fwww.yasinuludag.com%2FPyQt%2FTutorial02%2FTutorial02_ListModel.py&session_token=73zf6MKmZs2haGV1oousQoMmgC98MTM4ODE0NDUwMUAxMzg4MDU4MTAx). It is from a tutorial. – Ilyes Ferchiou Dec 26 '13 at 13:01
  • As written several times by now, returning True is only for the caller. It has no relevance to the update at all. It will only update if you emit the signal. If you do not, it will not. – László Papp Dec 26 '13 at 13:03
  • If you give a look to the whole code, you will see, it has 4 views based on the same model. As I already said, if I double click on an item in let's say view1 and modify it, it will get updated in that view (view1) even if I don't emit the signal ! However the 3 other views won't update until I set the focus on them with the mouse. So the signal just alerts the other views that the model has changed but it doesn't change what happens to the view from which the modification happened. – Ilyes Ferchiou Dec 26 '13 at 13:08
  • @IlyesFerchiou: I think I will move on. :) I already wrote all you need to understand for this. – László Papp Dec 26 '13 at 13:12
  • @IlyesFerchiou: I am not sure what is unclear in the code of the tutorial. It does use self.__colors in the rowCount(), data(), etc. Obviously, if you change it, you will have more rows, data, etc, so you will need to use that when you want to update, and you have to return False before having any such statement if any. – László Papp Dec 26 '13 at 13:31
1

I couldn't find much info on this. That said, this forum post by a "Qt Specialist" suggests that this behavior is a design choice by the Qt developers:

http://qt-project.org/forums/viewthread/31462

More specifically, the views do not lose your input if it gets rejected by the model. This may seem weird in the context of the tutorial you are following (where the color changes out of sync with the model), but in some contexts this might be desirable.

As an example, let's say you have designed a form using QLineEdits and QDataWidgetMapper to map the contents of the form to your model. Let's also suppose that QDataWidgetMapper::SubmitPolicy is set to AutoSubmit. In AutoSubmit mode, every time the QLineEdit is edited and then loses focus, the model is updated. If the model also rejects the change, and the current data (sans change) is repopulated into the QLineEdit, this would have the effect of the user having to start over (rather than fixing up their entry).

The alternative design choice is to allow the view to be out of sync with the model and, if this is undesirable, to push the responsibility to change this behavior onto the programmer.

Two ways that I can think of off-hand to change this behavior would be to:

  1. Create a custom delegate that handles setData -> false by emitting a custom dataRejected signal that the view could connect to and use to update itself.
  2. Create a "stateless" view for your model: force the view to retrieve data from the model any time it updates itself. In this manner, submitting a potential change to the model will not update the view unless that model emits a dataChanged signal, in which case the view will retrieve current state and update itself.
Terrabits
  • 997
  • 2
  • 15
  • 19