Closed altendky closed 3 years ago
BryceBeagle
commented
3 years ago Hmm that's a pretty good point and certainly a common use case. Is there any way to tell mypy that something should usually be not None, but it's technically possible for it to be None?
This situation is making me think of GCC's __builtin_expect or glibc/the Linux kernel's likely/unlikely macros. Assume one path, fall back to the other if it fails.
The-Compiler
commented
3 years ago I'm not aware of one - if we tell mypy that it can be None, it will require handling it properly everywhere. If we don't tell mypy about it, I suppose it'll complain with if QApplication.instance() is None...
altendky
commented
3 years ago Just to have the original situation that made me notice this listed here... It's nothing beautiful, but so it goes with global persistent state.
def maybe_build_application() -> QtCore.QCoreApplication:
"""Create a new Qt application object if one does not already exist.
Returns:
The Qt application object.
"""
application: QtCore.QCoreApplication
# TODO: https://bugreports.qt.io/browse/PYSIDE-1467
if qtpy.PYQT5:
maybe_application = QtWidgets.QApplication.instance()
elif qtpy.PYSIDE2:
maybe_application = typing.cast(
typing.Optional[QtCore.QCoreApplication], QtWidgets.QApplication.instance()
)
else: # pragma: no cover
raise qtrio.InternalError(
"You should not be here but you are running neither PyQt5 nor PySide2.",
)
if maybe_application is None:
application = QtWidgets.QApplication(sys.argv[1:])
else:
application = maybe_application
application.setQuitOnLastWindowClosed(False)
return applicationIf you don't want to check (I understand that) it could be a cast instead of an assert. Not that that changes much. There could be an exception-raising wrapper function that always returns an instance that is called instead. I hesitate to intentionally make hints wrong especially if the cost is touching just twenty bits of code in a fairly simple way with multiple options. If it is relevant, I'd be perfectly happy to do a PR for qutebrowser to respond to this change. I'll look around in case there's some other way to tell mypy to ignore the optionality.
While this is true in theory, I wonder if it's worth the pain it causes in practice? In qutebrowser, with this change I get:
For all those, I know a QApplication exists at that point. I'd now have to replace all occurrences of
QApplication.instance().something()with:to appease mypy, for very little benefit (if there was no QApplication, my application would crash at runtime either way).