Create a QApplication for inputhook if one doesn't already exist #9789

Merged
merged 1 commit into from Jul 25, 2016

Projects

None yet

3 participants

@takluyver
Member

Closes gh-9784

@takluyver takluyver Create a QApplication for inputhook if one doesn't already exist
Closes gh-9784
3a42b33
@takluyver takluyver added this to the 5.1 milestone Jul 21, 2016
@takluyver takluyver removed this from the 5.1 milestone Jul 21, 2016
@takluyver
Member

See discussion on #9784 before merging this.

@Carreau Carreau commented on the diff Jul 21, 2016
IPython/terminal/pt_inputhooks/qt.py
app = QtCore.QCoreApplication.instance()
if not app:
- return
@Carreau
Carreau Jul 21, 2016 Member

Want to drop a DeprecationWarning/(UserWarning maybe) here if App does not exists ? so that we can remove it at some point later?

@takluyver
takluyver Jul 21, 2016 Member

If we do decide to keep it in place, I wouldn't really consider it deprecated.

@Carreau
Carreau Jul 21, 2016 edited Member

I think we broke the use case in 5.0, and I'm fine restoring the behavior in next minor/patch version {especially since LTS]
I don't think in the long run we should create the App if it does not exists.

So I would reinstate it to not break user-code/habits. But warn about it, and remove it in the Py3Only-branch.

@Carreau Carreau added this to the 5.1 milestone Jul 22, 2016
@Carreau Carreau merged commit f4d0d99 into ipython:master Jul 25, 2016

3 checks passed

codecov/patch Coverage not affected when comparing 41ab88e...3a42b33
Details
codecov/project 74.27% (+0.00%) compared to 41ab88e
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tacaswell tacaswell commented on the diff Jul 29, 2016
IPython/terminal/pt_inputhooks/qt.py
@@ -1,10 +1,15 @@
import sys
from IPython.external.qt_for_kernel import QtCore, QtGui
+# If we create a QApplication, keep a reference to it so that it doesn't get
+# garbage collected.
+_appref = None
@tacaswell
tacaswell Jul 29, 2016 Contributor

90% sure that pyqt/pyside already do this.

@takluyver
takluyver Jul 29, 2016 Member

At least one of the options we support seemingly doesn't - I tried without it and things went wrong. I can't remember which one I was testing with, though.

@tacaswell
Contributor

@Carreau I disagree about not creating this the App. If the user has asked to use Qt event loop integration, then clearly they want to use Qt 😉 .

The qApp is a enforced-by-segfault singleton at the c++ level (the official line is creating more than one QApplication in the same process is undefined behavior) so all libraries that need a qApp need to go through this 'get the global if None create' dance (mpl does it here )

Having IPython do this as part of the integration is a big convenience for users.

@takluyver takluyver deleted the takluyver:i9784 branch Jul 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment