-
-
Notifications
You must be signed in to change notification settings - Fork 375
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
Fix QSocketNotifier in the Qt event loop not being disabled for the control channel #525
Fix QSocketNotifier in the Qt event loop not being disabled for the control channel #525
Conversation
…ontrol channel This was generating a constant warning in qtconsole and Spyder.
Hi @ccordoba12! Is this good to go? |
# notifier in order to connect a new one in the next | ||
# execution. This applies to the control channel. | ||
notifier.setEnabled(False) | ||
kernel.app.quit() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't putting this in an else
make the entire if/else
block unnecessary (both True/False cases run the exact same code)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about that too, but the if
also flushes the shell channel. So perhaps I should rewrite this to
stream.flush(limit=1)
notifier.setEnabled(False)
kernel.app.quit()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this meant to stop the repeated issuing of the QSocketNotifier warning or does it stop it from appearing completely?
It stops it completely because now we disable the previous notifier, so that a new one can be activated for the same Zmq socket. The problem is that, according to the Qt docs:
So, since flushing the control channel didn't return anything (because it's an extra channel that for now it's only used by the JupyterLab debugger, I think), we were not deactivating the previous notifier. And that triggered the warning. |
Released as 5.3.3. |
Thanks for your help @blink1073! Do you want me to push another PR to improve the code according to @djhoese's suggestions? I had them prepared but I was busy with other things, sorry. |
Pinging @anaconda-pkg-build about this one. It'd be really great if you could include this version in the upcoming Anaconda release. Thanks! |
Hi, I'm not sure whether this is the right place for reporting issues, but my qt in jupyter doesn't work any more after I upgraded ipykernel today from 5.3.0 to 5.3.3. Downgrading it back to 5.3.0 solves my problem, so I believe there's probably some incompatibility in this 5.3.3 version. Here is what I got: Test this code in jupyter notebook:
|
Installed ipykernel==5.3.3 and reinstalled spyder 4.1.4 and that seemed to fix the socket issue, so far. Thanks |
Hi @jacky4eyes, can you please open a new issue? I'm also curious if installing |
Hi all, import matplotlib.pyplot as plt
fig = plt.figure()
plt.show() However there is an interesting thing about it: Name Version Build Channel
I am not sure anymore, but it could be the case that this error showed up after i got a promt (by anaconda) to update conda navigator. (And did update it) SolutionAs found under the commit tap, adding the else loop part also helps in this case. |
I have alot of questions about your setup. It seems you are maybe mixing anaconda base with conda-forge stuff and are likely using old versions of certain packages. I'm not too sure, but it would help if we could get more organized details about your setup. |
This was generating a flush of warnings in qtconsole and Spyder.
Fixes jupyter/qtconsole#332
Fixes spyder-ide/spyder#13275