mottosso / Qt.py

Minimal Python 2 & 3 shim around all Qt bindings - PySide, PySide2, PyQt4 and PyQt5.
MIT License
896 stars 252 forks source link

QtCompat.isValid() Not Available for All Qt Objects #369

Closed andryjong closed 2 years ago

andryjong commented 2 years ago

QtCompat.isValid() starts with the following assert statement: https://github.com/mottosso/Qt.py/blob/242ec430ff390cf24051819802b9eec1e665d3b2/Qt.py#L816

However, not all objects in Qt inherits from QObject (e.g. QGraphicsItem, QListWidgetItem, QTreeWidgetItem, etc.). This assert statement makes QtCompat.isValid() not available for such objects.

How can I best check for object validity in this case?

mottosso commented 2 years ago

Hm, good question.

The reason to check isValid on a QObject is to avoid accessing memory that no longer exist. Could it be the case that these "items" do not allocate memory, and thus accessing members on them is always safe?

Looking at the members of QGraphicsItem, it looks like it can be cast to a QGraphicsObject (via toGraphicsObject) which does appear to inherit from QObject. Could it be the case that we need to explicitly call these?

andryjong commented 2 years ago

@mottosso -- Unfortunately, just like QObject, we cannot guarantee that an item is still in memory.

Casting QGraphicsItem into QGraphicsObject would work if the item itself is still valid. Otherwise, we'll get into the same situation where it would say something like "Internal C++ object already deleted". 😭

I've gotten around the AssertionError by having all of my items also inherit from QObject, like follows:

class GroupItem(QtWidgets.QGraphicsItemGroup, QtCore.QObject)

... but this feels rather like a "hacky" way to get around the assert statement.

mottosso commented 2 years ago

Oh, is the problem only that there is an assert in Qt.py? Then it is surely safe to remove. I can't remember why this was put there. What kind of error should we expect shiboken to throw if an object does not support it? Should probably be fine? Something the user could catch?

If that's the only problem, then a PR removing this assert is more than OK.

andryjong commented 2 years ago

@mottosso -- Is there a time frame and/or process to get the latest version to PyPI?

mottosso commented 2 years ago

Sorry, I'm working on it. It's supposed to be automatic, but the build chain (Travis, or PyPI) has changed, apparently. If it doesn't work in the next 20 mins, then it'll be tomorrow.

andryjong commented 2 years ago

Understood. Thank you!! 🙏

mottosso commented 2 years ago

All done! 🥳

andryjong commented 2 years ago

Thank you so much, sir!