diff options
author | Alex Hughes <ahughesalex@gmail.com> | 2018-04-08 13:45:41 -0700 |
---|---|---|
committer | Christian Tismer <tismer@stackless.com> | 2020-09-08 17:57:33 +0200 |
commit | c7904338f8707a30c70f1ddf62ec740cae255f36 (patch) | |
tree | a3c6bffb2fdeffc6885bd34a324ae391dbb205e4 /sources | |
parent | c5d47637c7376358a10f0f845e443478058a5430 (diff) |
Implement default __ne__ and __eq__ for all PySide types
PySide types have been following the Qt implementation of
comparisons, completely.
This is not correct for Python, because the Python default has
the operators `==` and `!=` at least. They are needed for tests
like `obj in collection`.
We fix this by redirecting the default case to
`PyBaseObject_Type.tp_richcompare`.
This is the correct way to fix it, because for types which do not
define `tp_richcompare', this is the default, anyway.
From the original patch, the test case is still in use.
Old message:
Implement __ne__ and __eq__ for QTreeWidgetItem
Testing if a QTreeWidgetItem belongs to a list raises a NotImplementedError.
I have exposed the operator== and the operator!= from C++ to shiboken which has solved our eq operator issue.
Implemented the test from PYSIDE-74 for the QTreeWidgetItem eq operator and the ne operator.
This also allows us to have the behavior "QTreeWidgetItem in ['a']" and "QTreeWidgetItem not in ['a']".
Adding qtreewidgetitem_test.py to CMakeFiles.txt
Fixes: PYSIDE-74
Change-Id: Id221c0163fc8c2d85730c4c26f22db5f61710706
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>
Diffstat (limited to 'sources')
-rw-r--r-- | sources/pyside2/doc/considerations.rst | 14 | ||||
-rw-r--r-- | sources/pyside2/tests/QtWidgets/CMakeLists.txt | 1 | ||||
-rw-r--r-- | sources/pyside2/tests/QtWidgets/qtreewidgetitem_test.py | 74 | ||||
-rw-r--r-- | sources/shiboken2/generator/shiboken2/cppgenerator.cpp | 2 | ||||
-rw-r--r-- | sources/shiboken2/libshiboken/basewrapper.cpp | 27 | ||||
-rw-r--r-- | sources/shiboken2/libshiboken/basewrapper.h | 3 | ||||
-rw-r--r-- | sources/shiboken2/tests/samplebinding/point_test.py | 5 |
7 files changed, 125 insertions, 1 deletions
diff --git a/sources/pyside2/doc/considerations.rst b/sources/pyside2/doc/considerations.rst index f05d929b5..b5eae7d86 100644 --- a/sources/pyside2/doc/considerations.rst +++ b/sources/pyside2/doc/considerations.rst @@ -133,3 +133,17 @@ Abandoned Alternative We also tried an alternative implementation with a ``qApp()`` function that was more *pythonic* and problem free, but many people liked the ``qApp`` macro better for its brevity, so here it is. + + +Rich Comparison +~~~~~~~~~~~~~~~ + +There was a long-standing bug in the ``tp_richcompare`` implementation of PySide classes. + +* When a class did not implement it, the default implementation of ``object`` is used. + This implements ``==`` and ``!=`` like the ``is`` operator. + +* When a class implements only a single function like ``<``, then the default implementation + was disabled, and expressions like ``obj in sequence`` failed with ``NotImplemented``. + +This oversight was fixed in version 5.15.1 . diff --git a/sources/pyside2/tests/QtWidgets/CMakeLists.txt b/sources/pyside2/tests/QtWidgets/CMakeLists.txt index 6682136ea..28372f437 100644 --- a/sources/pyside2/tests/QtWidgets/CMakeLists.txt +++ b/sources/pyside2/tests/QtWidgets/CMakeLists.txt @@ -126,6 +126,7 @@ PYSIDE_TEST(qtextedit_test.py) PYSIDE_TEST(qtextedit_signal_test.py) PYSIDE_TEST(qtreeview_test.py) PYSIDE_TEST(qtreewidget_test.py) +PYSIDE_TEST(qtreewidgetitem_test.py) PYSIDE_TEST(qtoolbar_test.py) PYSIDE_TEST(qtoolbox_test.py) PYSIDE_TEST(qvariant_test.py) diff --git a/sources/pyside2/tests/QtWidgets/qtreewidgetitem_test.py b/sources/pyside2/tests/QtWidgets/qtreewidgetitem_test.py new file mode 100644 index 000000000..115bce4b7 --- /dev/null +++ b/sources/pyside2/tests/QtWidgets/qtreewidgetitem_test.py @@ -0,0 +1,74 @@ +#!/usr/bin/python + +############################################################################# +## +## Copyright (C) 2020 The Qt Company Ltd. +## Contact: https://www.qt.io/licensing/ +## +## This file is part of the test suite of PySide2. +## +## $QT_BEGIN_LICENSE:GPL-EXCEPT$ +## Commercial License Usage +## Licensees holding valid commercial Qt licenses may use this file in +## accordance with the commercial license agreement provided with the +## Software or, alternatively, in accordance with the terms contained in +## a written agreement between you and The Qt Company. For licensing terms +## and conditions see https://www.qt.io/terms-conditions. For further +## information use the contact form at https://www.qt.io/contact-us. +## +## GNU General Public License Usage +## Alternatively, this file may be used under the terms of the GNU +## General Public License version 3 as published by the Free Software +## Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT +## included in the packaging of this file. Please review the following +## information to ensure the GNU General Public License requirements will +## be met: https://www.gnu.org/licenses/gpl-3.0.html. +## +## $QT_END_LICENSE$ +## +############################################################################# + +""" +Unit tests for QTreeWidgetItem +------------------------------ + +This test is actually meant for all types which provide `tp_richcompare` +but actually define something without providing `==` or `!=` operators. +QTreeWidgetItem for instance defines `<` only. + +PYSIDE-74: We redirect to type `object`s handling which is anyway the default + when `tp_richcompare` is undefined. +""" + +import os +import sys +import unittest + +sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from init_paths import init_test_paths +init_test_paths(False) + +from PySide2 import QtCore, QtWidgets + + +class QTreeWidgetItemTest(unittest.TestCase): + def testClass(self): + app = QtWidgets.QApplication([]) + treewidget = QtWidgets.QTreeWidget() + item = QtWidgets.QTreeWidgetItem(["Words and stuff"]) + item2 = QtWidgets.QTreeWidgetItem(["More words!"]) + treewidget.insertTopLevelItem(0, item) + + dummy_list = ["Numbers", "Symbols", "Spam"] + self.assertFalse(item in dummy_list) + self.assertTrue(item not in dummy_list) + self.assertFalse(item == item2) + self.assertTrue(item != item2) + treewidget.show() + QtCore.QTimer.singleShot(500, app.quit) + app.exec_() + + +if __name__ == "__main__": + unittest.main() + diff --git a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp index 73f1a10cf..8465224bb 100644 --- a/sources/shiboken2/generator/shiboken2/cppgenerator.cpp +++ b/sources/shiboken2/generator/shiboken2/cppgenerator.cpp @@ -4613,6 +4613,8 @@ void CppGenerator::writeRichCompareFunction(QTextStream &s, const GeneratorConte s << INDENT << "default:\n"; { Indentation indent(INDENT); + s << INDENT << "// PYSIDE-74: By default, we redirect to object's tp_richcompare (which is `==`, `!=`).\n"; + s << INDENT << "return FallbackRichCompare(self, " << PYTHON_ARG << ", op);\n"; s << INDENT << "goto " << baseName << "_RichComparison_TypeError;\n"; } } diff --git a/sources/shiboken2/libshiboken/basewrapper.cpp b/sources/shiboken2/libshiboken/basewrapper.cpp index 961c6c739..decfd01db 100644 --- a/sources/shiboken2/libshiboken/basewrapper.cpp +++ b/sources/shiboken2/libshiboken/basewrapper.cpp @@ -823,6 +823,33 @@ PyObject *SbkType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases) return type; } +// PYSIDE-74: Fallback used in all types now. +PyObject *FallbackRichCompare(PyObject *self, PyObject *other, int op) +{ + // This is a very simple implementation that supplies a simple identity. + static const char * const opstrings[] = {"<", "<=", "==", "!=", ">", ">="}; + PyObject *res; + + switch (op) { + + case Py_EQ: + res = (self == other) ? Py_True : Py_False; + break; + case Py_NE: + res = (self != other) ? Py_True : Py_False; + break; + default: + PyErr_Format(PyExc_TypeError, + "'%s' not supported between instances of '%.100s' and '%.100s'", + opstrings[op], + self->ob_type->tp_name, + other->ob_type->tp_name); + return NULL; + } + Py_INCREF(res); + return res; +} + } //extern "C" diff --git a/sources/shiboken2/libshiboken/basewrapper.h b/sources/shiboken2/libshiboken/basewrapper.h index b233c02b4..267759daa 100644 --- a/sources/shiboken2/libshiboken/basewrapper.h +++ b/sources/shiboken2/libshiboken/basewrapper.h @@ -135,6 +135,9 @@ LIBSHIBOKEN_API PyObject *SbkDummyNew(PyTypeObject *type, PyObject *, PyObject * LIBSHIBOKEN_API PyObject *SbkType_FromSpec(PyType_Spec *); LIBSHIBOKEN_API PyObject *SbkType_FromSpecWithBases(PyType_Spec *, PyObject *); +/// PYSIDE-74: Fallback used in all types now. +LIBSHIBOKEN_API PyObject *FallbackRichCompare(PyObject *self, PyObject *other, int op); + } // extern "C" namespace Shiboken diff --git a/sources/shiboken2/tests/samplebinding/point_test.py b/sources/shiboken2/tests/samplebinding/point_test.py index e2beb78ae..36e6bc642 100644 --- a/sources/shiboken2/tests/samplebinding/point_test.py +++ b/sources/shiboken2/tests/samplebinding/point_test.py @@ -71,7 +71,10 @@ class PointTest(unittest.TestCase): '''Test Point class != operator.''' pt1 = Point(5.0, 2.3) pt2 = Point(5.0, 2.3) - self.assertRaises(NotImplementedError, pt1.__ne__, pt2) + # This test no longer makes sense because we always supply default `==`, `!=`. + #self.assertRaises(NotImplementedError, pt1.__ne__, pt2) + # Since we use the default identity comparison, this results in `!=` . + self.assertTrue(pt1 != pt2) def testReturnNewCopy(self): '''Point returns a copy of itself.''' |