[PyQt] 2 Updates for the modeltest in PyQt4
Hi, just started a new project with PyQt which will involve a model, hence I've sat down and checked wether the python-version of the modeltest is up-to-date with the C++ version. There turned out to be one bug (broken signal names) and the C++ version now also checks the layoutChanged signal. Attached two patches fix that. I didn't test yet against an actual model, so might have done something wrong in porting. Andreas -- You have the body of a 19 year old. Please return it before it gets wrinkled. >From f78e30900d928f2e87202509e9ef0ddd14c634fa Mon Sep 17 00:00:00 2001 From: Andreas Pakulat Date: Sun, 20 Feb 2011 21:56:40 +0100 Subject: [PATCH 1/2] Fix signal names. The PyQt documentation does not indicate that these signals have been renamed from their C++ version, so the testing of them was broken. --- tests/modeltest/modeltest.py |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/modeltest/modeltest.py b/tests/modeltest/modeltest.py index 64f0511..e2f7a9a 100644 --- a/tests/modeltest/modeltest.py +++ b/tests/modeltest/modeltest.py @@ -40,7 +40,7 @@ class ModelTest(QtCore.QObject): self.connect( self.model, QtCore.SIGNAL("columnsAboutToBeInserted(const QModelIndex&, int, int)"), self.runAllTests) self.connect( self.model, QtCore.SIGNAL("columnsAboutToBeRemoved(const QModelIndex&, int, int)"), self.runAllTests) -self.connect( self.model, QtCore.SIGNAL("columnsBeInserted(const QModelIndex&, int, int)"), self.runAllTests) +self.connect( self.model, QtCore.SIGNAL("columnsInserted(const QModelIndex&, int, int)"), self.runAllTests) self.connect( self.model, QtCore.SIGNAL("columnsRemoved(const QModelIndex&, int, int)"), self.runAllTests) self.connect( self.model, QtCore.SIGNAL("dataChanged(const QModelIndex&, const QModelIndex&)"), self.runAllTests) self.connect( self.model, QtCore.SIGNAL("headerDataChanged(Qt::Orientation, int, int)"), self.runAllTests) @@ -49,13 +49,13 @@ class ModelTest(QtCore.QObject): self.connect( self.model, QtCore.SIGNAL("modelReset()"), self.runAllTests) self.connect( self.model, QtCore.SIGNAL("rowsAboutToBeInserted(const QModelIndex&, int, int)"), self.runAllTests) self.connect( self.model, QtCore.SIGNAL("rowsAboutToBeRemoved(const QModelIndex&, int, int)"), self.runAllTests) -self.connect( self.model, QtCore.SIGNAL("rowsBeInserted(const QModelIndex&, int, int)"), self.runAllTests) +self.connect( self.model, QtCore.SIGNAL("rowsInserted(const QModelIndex&, int, int)"), self.runAllTests) self.connect( self.model, QtCore.SIGNAL("rowsRemoved(const QModelIndex&, int, int)"), self.runAllTests) # Special checks for inserting/removing self.connect( self.model, QtCore.SIGNAL("rowsAboutToBeInserted(const QModelIndex&, int, int)"), self.rowsAboutToBeInserted) self.connect( self.model, QtCore.SIGNAL("rowsAboutToBeRemoved(const QModelIndex&, int, int)"), self.rowsAboutToBeRemoved) -self.connect( self.model, QtCore.SIGNAL("rowsBeInserted(const QModelIndex&, int, int)"), self.rowsInserted) +self.connect( self.model, QtCore.SIGNAL("rowsInserted(const QModelIndex&, int, int)"), self.rowsInserted) self.connect( self.model, QtCore.SIGNAL("rowsRemoved(const QModelIndex&, int, int)"), self.rowsRemoved) self.runAllTests() -- 1.7.2.3 >From c7fb450a7da01dac6fb7351efb547af4ceba97d6 Mon Sep 17 00:00:00 2001 From: Andreas Pakulat Date: Sun, 20 Feb 2011 21:57:25 +0100 Subject: [PATCH 2/2] Add new check for layout-changes. This is a test that has been added to the C++ version at some point, it verifies the first 100 rows to be changing in the correct way when the layout changes. --- tests/modeltest/modeltest.py | 13 + 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/tests/modeltest/modeltest.py b/tests/modeltest/modeltest.py index e2f7a9a..d0c6e12 100644 --- a/tests/modeltest/modeltest.py +++ b/tests/modeltest/modeltest.py @@ -35,6 +35,7 @@ class ModelTest(QtCore.QObject): self.model = sip.cast(_model, QtCore.QAbstractItemModel) self.insert = [] self.remove = [] +self.changing = [] self.fetchingMore = False assert(self.model) @@ -53,6 +54,9 @@ class ModelTest(QtCore.QObject): self.connect( self.model, QtCore.SIGNAL("rowsRemoved(const QModelIndex&, int, int)"), self.runAllTests) # Special checks for inserting/removing +self.connect( self.model, QtCore.SIGNAL("layoutAboutToBeChanged()"), self.layoutAboutToBeChanged ) +self.connect( self.model, QtCore.SIGNAL("layoutChanged()"), self.layoutChanged ) + self.connect( self.model, QtCore.SIGNAL("rowsAboutToBeInserted(const QModelIndex&, int, int)"), self.rowsAboutToBeInserted) self.connect( self.model, QtCore.SIGNAL("rowsAboutToBeRemoved(const QModelIndex&, int, int)"), self.rowsAboutToBeRemoved) self.
Re: [PyQt] 2 Updates for the modeltest in PyQt4
On Sonntag 20 Februar 2011, Andreas Pakulat wrote: > I didn't test yet against an actual model, so might have done > something wrong in porting. yep. lists have no add, it is append I didn't test either but pylint did Where do you maintain modeltest.py? I got it from http://bazaar.launchpad.net/~bzr/ubuntu/maverick/qbzr/bzr- ppa/annotate/head:/lib/tests/modeltest.py I made a few bug fixes and I believe I mailed them back to the author but now that I check again not all of them made it in, there are still some self. missing in front of model.* also, checking values for TextAlignmentRole is still wrong there: it does not accept ORed values I also changed some things to make pylint happy (my local git commit hook does not let me commit unless pylint is silent) my version, now with your patches: svn cat svn://anonsvn.kde.org/home/kde/trunk/KDE/kdegames/kajongg/src/modeltest.py there also is a modeltest.py from riverbank in contrib/pymodeltest but it has a lot of missing self and does not contain your patches. your paths look like you got it from somewere else again: tests/modeltest/modeltest.py so I know about three different copies where I believe mine has less bugs than the others, and you seem to be patching a forth copy -- Wolfgang ___ PyQt mailing listPyQt@riverbankcomputing.com http://www.riverbankcomputing.com/mailman/listinfo/pyqt
Re: [PyQt] 2 Updates for the modeltest in PyQt4
On 20.02.11 23:52:13, Wolfgang Rohdewald wrote: > On Sonntag 20 Februar 2011, Andreas Pakulat wrote: > > I didn't test yet against an actual model, so might have done > > something wrong in porting. > > yep. > > lists have no add, it is append Heh, worked too much with sets lately. > I didn't test either but pylint did > > Where do you maintain modeltest.py? In the repository that I develop that project in. Its completely private at this point. > I got it from > http://bazaar.launchpad.net/~bzr/ubuntu/maverick/qbzr/bzr- > ppa/annotate/head:/lib/tests/modeltest.py > > I made a few bug fixes and I believe I mailed them back to > the author but now that I check again not all of them made > it in, there are still some self. missing in front of model.* > > also, checking values for TextAlignmentRole is still wrong > there: it does not accept ORed values Interesting, I didn't spot any differences between the C++ and Python version in that area. Then again I didn't look too close either... > I also changed some things to make pylint happy > (my local git commit hook does not let me commit unless pylint is > silent) > > my version, now with your patches: > svn cat > svn://anonsvn.kde.org/home/kde/trunk/KDE/kdegames/kajongg/src/modeltest.py Thanks for letting me know, will check that version out. > there also is a modeltest.py from riverbank in > contrib/pymodeltest > but it has a lot of missing self and does not contain your > patches. > > your paths look like you got it from somewere else again: > tests/modeltest/modeltest.py > > so I know about three different copies where > I believe mine has less bugs than the others, and you > seem to be patching a forth copy I'm patching PyQt4 4.8.3's version currently and hence posted the patches here. Andreas -- Time to be aggressive. Go after a tattooed Virgo. ___ PyQt mailing listPyQt@riverbankcomputing.com http://www.riverbankcomputing.com/mailman/listinfo/pyqt
Re: [PyQt] 2 Updates for the modeltest in PyQt4
On 20.02.11 23:52:13, Wolfgang Rohdewald wrote: > On Sonntag 20 Februar 2011, Andreas Pakulat wrote: > > I didn't test yet against an actual model, so might have done > > something wrong in porting. > > yep. > > lists have no add, it is append > > I didn't test either but pylint did > > Where do you maintain modeltest.py? > > I got it from > http://bazaar.launchpad.net/~bzr/ubuntu/maverick/qbzr/bzr- > ppa/annotate/head:/lib/tests/modeltest.py [...] > my version, now with your patches: > svn cat > svn://anonsvn.kde.org/home/kde/trunk/KDE/kdegames/kajongg/src/modeltest.py Now did a closer look at the diff between that version of mine, the attached 3 patches (one is the add vs. append change) contain the changes. > I made a few bug fixes and I believe I mailed them back to > the author but now that I check again not all of them made > it in, there are still some self. missing in front of model.* Couldn't find any of those, just a self. missing in front of insert. > also, checking values for TextAlignmentRole is still wrong > there: it does not accept ORed values Can you elaborate on this? > I also changed some things to make pylint happy > (my local git commit hook does not let me commit unless pylint is > silent) Thats probably all those name-changes and line-breaks etc? There's one change that I don't quite understand though. Its the "Check that we can get back our real parent", your version does: parentIdx = self.model.parent( index ) assert( parentIdx.internalId() == parent.internalId() ) assert( parentIdx.row() == parent.row() ) while the PyQt version just compares the index against the stored one. Do you have any idea why that was done in qbzr (the log message there is less than useful). Andreas -- Living your life is a task so difficult, it has never been attempted before. >From 26ee4e9e79b87987a530434dd82d439eebf47701 Mon Sep 17 00:00:00 2001 From: Andreas Pakulat Date: Mon, 21 Feb 2011 21:37:20 +0100 Subject: [PATCH 1/3] Fix running, lists only know append --- tests/modeltest/modeltest.py |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tests/modeltest/modeltest.py b/tests/modeltest/modeltest.py index d0c6e12..9fde497 100644 --- a/tests/modeltest/modeltest.py +++ b/tests/modeltest/modeltest.py @@ -347,7 +347,7 @@ class ModelTest(QtCore.QObject): def layoutAboutToBeChanged(self): for i in range(0, max(0, min( self.model.rowCount(), 100))): -self.changing.add( QtCore.QPersistentModelIndex( self.model.index( i, 0 ) ) ) +self.changing.append( QtCore.QPersistentModelIndex( self.model.index( i, 0 ) ) ) def layoutChanged(self): for c in self.changing: -- 1.7.2.3 >From 500bdf421c34e9a2c4f92c871ce3eb7b776e5029 Mon Sep 17 00:00:00 2001 From: Andreas Pakulat Date: Mon, 21 Feb 2011 21:37:54 +0100 Subject: [PATCH 2/3] Fix this assert to not automatically trigger Apparently sip.cast creates a new python object each time its invoked. This changes the id of the object and hence the comparison fails here. So lets use the original instance of the model we store in self._model for comparison here. We can't leave away the sip.cast because some model subclasses re-define visibility of public API of QAbstractItemModel. (QStringListModel makes columnCount private). --- tests/modeltest/modeltest.py |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/tests/modeltest/modeltest.py b/tests/modeltest/modeltest.py index 9fde497..fefe5b4 100644 --- a/tests/modeltest/modeltest.py +++ b/tests/modeltest/modeltest.py @@ -419,7 +419,9 @@ class ModelTest(QtCore.QObject): assert( a == b ) # Some basic checking on the index that is returned -assert( index.model() == self.model ) +# This raises an error that is not part of the qbzr code. +# see http://www.opensubscriber.com/message/pyqt@riverbankcomputing.com/10335500.html +assert( index.model() == self._model ) assert( index.row() == r ) assert( index.column() == c ) # While you can technically return a QtCore.QVariant usually this is a sign -- 1.7.2.3 >From ec34a8a07f317baffc02d842a6d82586be3020d8 Mon Sep 17 00:00:00 2001 From: Andreas Pakulat Date: Mon, 21 Feb 2011 21:41:36 +0100 Subject: [PATCH 3/3] Add self here. Thats necessary as the change stored in aboutToBeInserted is being used after the insertion has taken place. --- tests/modeltest/modeltest.py |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/modeltest/modeltest.py b/tests/modeltest/modeltest.py index fefe5b4..442eb9b 100644 --- a/tests/modeltest/modeltest.py +++ b/tests/modeltest/modeltest.py @@ -305,13 +305,13 @@ class ModelTest(QtCore.QObject): c['oldSize'] = self.model.rowCount(parent) c['last'] = self.model.data(model.index(start-1, 0, parent)) c['next'] = self.model.data(model.i
Re: [PyQt] 2 Updates for the modeltest in PyQt4
On Montag 21 Februar 2011, Andreas Pakulat wrote: > Now did a closer look at the diff between that version of > mine, the attached 3 patches (one is the add vs. append > change) contain the changes. > > > I made a few bug fixes and I believe I mailed them back to > > the author but now that I check again not all of them made > > it in, there are still some self. missing in front of > > model.* > > Couldn't find any of those, just a self. missing in front of > insert. in rowsAboutToBeInserted and rowsInserted, rowsAboutToBeRemoved and rowsRemoved, several places similar to self.model.data(model.index(... that should be self.model.index > > also, checking values for TextAlignmentRole is still wrong > > there: it does not accept ORed values > > Can you elaborate on this? diff between launchpad.net file and mine: -assert( alignment == QtCore.Qt.AlignLeft or -alignment == QtCore.Qt.AlignRight or -alignment == QtCore.Qt.AlignHCenter or -alignment == QtCore.Qt.AlignJustify) +assert( alignment == (alignment & int(QtCore.Qt.AlignHorizontal_Mask | QtCore.Qt.AlignVertical_Mask))) and c++: Q_ASSERT ( alignment == ( alignment & ( Qt::AlignHorizontal_Mask | Qt::AlignVertical_Mask ) ) ); the launchpad.net version would not accept Qt.AlignLeft|Qt.AlignVCenter the C++ version and mine do. > > I also changed some things to make pylint happy > > (my local git commit hook does not let me commit unless > > pylint is silent) > > Thats probably all those name-changes and line-breaks etc? yes. And whitespace changes and some docstrings. Maybe it is easier to look at my single commits for spotting relevant differences. > There's one change that I don't quite understand though. Its > the "Check that we can get back our real parent", your > version does: > > parentIdx = self.model.parent( index ) > assert( parentIdx.internalId() == parent.internalId() ) > assert( parentIdx.row() == parent.row() ) > > while the PyQt version just compares the index against the > stored one. Do you have any idea why that was done in qbzr > (the log message there is less than useful). I have no idea. In Kajongg I have an editable treeview and adding assert(self.model.parent(index) == parent) passes (just tried). You'll have to ask the author -- Wolfgang ___ PyQt mailing listPyQt@riverbankcomputing.com http://www.riverbankcomputing.com/mailman/listinfo/pyqt