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

show_info() not working when running napari from a Jupyter notebook #6455

Closed
LucaMarconato opened this issue Nov 15, 2023 · 9 comments · Fixed by #6882
Closed

show_info() not working when running napari from a Jupyter notebook #6455

LucaMarconato opened this issue Nov 15, 2023 · 9 comments · Fixed by #6882
Labels
bug Something isn't working

Comments

@LucaMarconato
Copy link
Contributor

🐛 Bug Report

show_info() doesn't work when I run napari from a Jupyter notebook. Everything is fine when I run napari from a script. Below is code to reproduce this.

💡 Steps to Reproduce

Run this code in a Jupyter notebook and the click anywhere on the image.

from napari.utils.notifications import show_info
import napari
from skimage import data

viewer = napari.Viewer()
layer = viewer.add_image(data.astronaut(), name='astro')

@layer.mouse_drag_callbacks.append
def update_tooltip(layer, event):
    show_info(f'{event.position}')
napari.run()

💡 Expected Behavior

On click, it should show the notification on the bottom like in the screenshot
image

🌎 Environment

napari: 0.4.16
Platform: macOS-13.4.1-arm64-arm-64bit
System: MacOS 13.4.1
Python: 3.9.18 | packaged by conda-forge | (main, Aug 30 2023, 03:53:08) [Clang 15.0.7 ]
Qt: 5.15.8
PyQt5: 5.15.9
NumPy: 1.24.4
SciPy: 1.9.1
Dask: 2023.10.1
VisPy: 0.10.0

OpenGL:

  • GL version: 2.1 Metal - 83.1
  • MAX_TEXTURE_SIZE: 16384

Screens:

  • screen 1: resolution 2056x1329, scale 2.0

Plugins:

  • napari-console: 0.0.9
  • napari-matplotlib: 1.2.1.dev2+g7b6a4a4
  • napari-spatialdata: 0.1.0
  • napari-svg: 0.1.10
  • scikit-image: 0.4.16

💡 Additional Context

No response

@LucaMarconato LucaMarconato added the bug Something isn't working label Nov 15, 2023
@Czaki
Copy link
Collaborator

Czaki commented Nov 15, 2023

Explanation:

Because jupyter starts ow qt event loop the get_app() called from napari.run() function does not connect NapariQtNotification dispatchers to notification_manager signals.

if not _ipython_has_eventloop():
notification_manager.notification_ready.connect(
NapariQtNotification.show_notification
)
notification_manager.notification_ready.connect(
show_console_notification
)

In meantime, we update the guard in NapariQtNotification.show_notification to not trigger a notification if there is no main window in #4839

@classmethod
@ensure_main_thread
def show_notification(cls, notification: Notification):
from napari._qt.qt_main_window import _QtMainWindow
from napari.settings import get_settings
settings = get_settings()
# after https://github.com/napari/napari/issues/2370,
# the os.getenv can be removed (and NAPARI_CATCH_ERRORS retired)
if (
notification.severity
>= settings.application.gui_notification_level
and _QtMainWindow.current()
):
canvas = _QtMainWindow.current()._qt_viewer._welcome_widget
cls.from_notification(notification, canvas).show()

So we may should revisit our design decision.

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Apr 30, 2024

I ran into this today. Pretty important use-case, particularly for keybindings, because printing prints to the Jupyter so it's not readily visible in napari if you are working on a smaller screen. Definitely should aim to fix this (lack of) interaction.

Edit: because it's not immediately apparent from the post above, it works from iPython because we specifically make the connections--I think. So we should aim to do the same for Jupyter.

OK, LOL, so it works from iPython but shouldn't.
Here's the _ipython_has_eventloop():

def _ipython_has_eventloop() -> bool:
"""Return True if IPython %gui qt is active.
Using this is better than checking ``QApp.thread().loopLevel() > 0``,
because IPython starts and stops the event loop continuously to accept code
at the prompt. So it will likely "appear" like there is no event loop
running, but we still don't need to start one.
"""
ipy_module = sys.modules.get("IPython")
if not ipy_module:
return False
shell: InteractiveShell = ipy_module.get_ipython() # type: ignore
if not shell:
return False
return shell.active_eventloop == 'qt'

in iPython I get:

In [9]: sys.modules.get("IPython").get_ipython().active_eventloop
Out[9]: 'qt5'

But the check, uses just 'qt'

return shell.active_eventloop == 'qt'

so it returns False. So then we have if not False and the connections are made, just like if shell was False

@Czaki Is there any workaround that could be used in the meanwhile?

@Czaki
Copy link
Collaborator

Czaki commented Apr 30, 2024

OK, LOL, so it works from iPython but shouldn't.
Here's the _ipython_has_eventloop():

Did you run ipython with qtbackend?

@Czaki Is there any workaround that could be used in the meanwhile?

Manually perform this steep?

     notification_manager.notification_ready.connect( 
         NapariQtNotification.show_notification 
     ) 

And we should check if we could perform bind always. if was introduced in #2633

@psobolewskiPhD
Copy link
Member

I just typed ipython in my terminal in an env with just napari and jupyterlab. So I'm not sure what backend, but it returned qt5 so safe to assume qt. i didn't run any magics.

@Czaki
Copy link
Collaborator

Czaki commented Apr 30, 2024

Ok. I see.
We should dig now if the if guard is still needed.

@psobolewskiPhD
Copy link
Member

What I don't get is why jupyter gets caught by that if
From jupyter, all that code works and shell.active_eventloop returns None, so the logic should fire, because shell.active_eventloop == 'qt' returns False

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented May 1, 2024

Dropping the if seems to work fine with jupyter and ipython, but I'm not sure how to test this better:
https://github.com/psobolewskiPhD/napari/tree/fix_notif_jupyter

@Czaki
Copy link
Collaborator

Czaki commented May 1, 2024

I think that you should make a PR with remove of this if. Then it will be present on main and maybe someone will report some problem if any exists.

@LucaMarconato
Copy link
Contributor Author

I have tried #6882 and the problem is solved! Thanks a lot!

@jni jni closed this as completed in #6882 May 15, 2024
jni pushed a commit that referenced this issue May 15, 2024
…om Jupyter (#6882)

# References and relevant issues
Closes #6455

# Description
The if statement removed was supposed to block ipython, but wasn't doing
that.
Dropping it so that notifications JustWork.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants