Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qt6 compatibility fixes for core PythonQt library #109

Merged

Conversation

richard42
Copy link
Contributor

No description provided.

@richard42 richard42 mentioned this pull request Aug 30, 2023
@@ -463,8 +463,8 @@ static PyObject *PythonQtClassWrapper_getattro(PyObject *obj, PyObject *name)
}
PyObject* dict = PyDict_New();

QSet<QString> completeSet = QSet<QString>::fromList(wrapper->classInfo()->memberList());
completeSet.unite(QSet<QString>::fromList(wrapper->classInfo()->propertyList()));
QSet<QString> completeSet(wrapper->classInfo()->memberList().begin(), wrapper->classInfo()->memberList().end());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both memberList() and propertyList() return a new QStringList object; it isn't a valid use to use begin() and end() from different list instances, even if it seems to work for you.
Assign the result of memberList() and propertyList() to a local variable and use begin() and end() on that.

@@ -1015,17 +1015,17 @@ QVariant PythonQtConv::PyObjToQVariant(PyObject* val, int type)
if (wrap->classInfo()->isCPPWrapper()) {
if (wrap->classInfo()->metaTypeId()>0) {
// construct a new variant from the C++ object if it has a meta type (this will COPY the object!)
v = QVariant(wrap->classInfo()->metaTypeId(), wrap->_wrappedPtr);
v = QVariant(QMetaType(wrap->classInfo()->metaTypeId()), wrap->_wrappedPtr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a note: It seems this constructor of QVariant only exists for Qt6, so we can't merge this as is into master, as this would break compatibility with Qt5.
I'd prefer to have the same code of the PythonQt library itself for Qt5 and Qt6 (the binding generator is a completely different thing), so we will need a few #if QT_VERSION >= 0x060000 statements. And we will need to decide which Qt5 versions we still want to support.

@@ -25,12 +25,16 @@ isEmpty(PYTHONQT_STATIC) {

DEFINES += PYTHONQT_CATCH_ALL_EXCEPTIONS

contains(QT_MAJOR_VERSION, 5) {
contains(QT_MAJOR_VERSION, 6) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: Perhaps use
contains(QT_MAJOR_VERSION, 5)|contains(QT_MAJOR_VERSION, 6)
when we achieve true compatibility with Qt5 and Qt6

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is for Qt5 and 6 only, there is no need for the check at all.

@mrbean-bremen mrbean-bremen merged commit cfa9a34 into MeVisLab:master Sep 15, 2023
0 of 11 checks passed
@mrbean-bremen
Copy link
Contributor

Sorry, accidentally merged that - have reverted it, but I created a new Qt6 branch which I rebased against your branch, to do further work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants