napari: Move layer triggers weird bug in viewer

🐛 Bug

When I want to move a layer, a strange bug is happening in the viewer. I don’t know if it’s related to my environment, if I am doing something wrong. But here is what happens: when I move a layer in the viewer, I get this error:

> ---------------------------------------------------------------------------
> AttributeError                            Traceback (most recent call last)
> /usr/local/lib/python3.10/site-packages/napari/_qt/containers/_layer_delegate.py in paint(self=<napari._qt.containers._layer_delegate.LayerDelegate object>, painter=<PyQt5.QtGui.QPainter object>, option=<PyQt5.QtWidgets.QStyleOptionViewItem object>, index=<PyQt5.QtCore.QModelIndex object>)
>      84         # update the icon based on layer type
>      85 
> ---> 86         self.get_layer_icon(option, index)
>         self.get_layer_icon = <bound method LayerDelegate.get_layer_icon of <napari._qt.containers._layer_delegate.LayerDelegate object at 0x7f193fc9b7f0>>
>         option = <PyQt5.QtWidgets.QStyleOptionViewItem object at 0x7f1901960040>
>         index = <PyQt5.QtCore.QModelIndex object at 0x7f18ffd2f450>
>      87         # paint the standard itemView (includes name, icon, and vis. checkbox)
>      88         super().paint(painter, option, index)
> 
> /usr/local/lib/python3.10/site-packages/napari/_qt/containers/_layer_delegate.py in get_layer_icon(self=<napari._qt.containers._layer_delegate.LayerDelegate object>, option=<PyQt5.QtWidgets.QStyleOptionViewItem object>, index=<PyQt5.QtCore.QModelIndex object>)
>      97             icon_name = 'folder-open' if expanded else 'folder'
>      98         else:
> ---> 99             icon_name = f'new_{layer._type_string}'
>         icon_name = undefined
>     100 
>     101         try:
> 
> AttributeError: 'NoneType' object has no attribute '_type_string'

In addition, there is the same error as #4420: the layer remains in the foreground, even if it has been moved from the last layer to a lower layer.

To Reproduce

Steps to reproduce the behavior:

Move layer from on old index to a new index viewer.layers.move(old_index, new_index)

Expected behavior

This error AttributeError: 'NoneType' object has no attribute '_type_string' should not be raised… And

Environment

napari: 0.4.15 Platform: Linux-5.17.4-200.fc35.x86_64-x86_64-with-glibc2.34 System: Fedora Linux 35 (Workstation Edition) Python: 3.10.4 (main, Mar 25 2022, 00:00:00) [GCC 11.2.1 20220127 (Red Hat 11.2.1-9)] Qt: 5.15.2 PyQt5: 5.15.6 NumPy: 1.21.5 SciPy: 1.8.0 Dask: 2022.04.0 VisPy: 0.9.6

OpenGL:

GL version: 4.6.0 NVIDIA 510.47.03
MAX_TEXTURE_SIZE: 16384

Screens:

screen 1: resolution 1920x1200, scale 1.0

Additional context

This bug appears in the context of the development of a new plugin (this logic is triggered in a widget). I have another bug here #4420 which might be related.

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 15 (11 by maintainers)

Commits related to this issue

Connect napari events first to EventEmitter (#4480) In some cases (#3337, #4421, #4425), connecting callbacks to napari events caused issues, because napari itself internally uses events to get its ... — committed to napari/napari by Czaki 2 years ago
Connect napari events first to EventEmitter (#4480) In some cases (#3337, #4421, #4425), connecting callbacks to napari events caused issues, because napari itself internally uses events to get its ... — committed to napari/napari by Czaki 2 years ago

Most upvoted comments

Fwiw, I’ve always disliked the position argument in general. I don’t include it in psygnal, and I think that, internally, napari shouldn’t care what has been connected or in what order. So @Czaki #4425 seems to be the more important issue here. (That is, it’s up to us to make sure that the event is only emitted after all internal state has stabilized)

tlambert03 on Apr 26, 2022

Here is the documentation: https://napari.org/api/napari.utils.events.EventEmitter.html#napari.utils.events.EventEmitter.connect

But it may not be clear where it is for someone who does not know the napari code.

A little more explanation. In napari, callback functions are stored on a list (you could connect multiple functions to one event) and executed in order from this list. So connect effectively is adding the callback to the list. And position controls if it is inserted on begin or the end.

I agree that it should be better implemented/documented.

Czaki on Apr 26, 2022

Default value for position is "first". So added callback is executed before earlier added. Changing to "last" make callback called after all earlier added callbacks.

The problem could happen, if someone added next callback on the end of callback list.

So if you set short time in Timer this should do job for you. Feel free to ask questions if documentation is not clear for you or you do not meet some concepts ealier.

Czaki on Apr 26, 2022

Ok. I think that the source of problems is that connect()inserts the connected function in the first position so your functions are called before all napari internal hooks. Then You modify the state of the layer list and all other callbacks are triggered with the wrong state.

Maybe simplest solution could be use position keyword argument, like this: v.layers.events.inserted.connect(move, position="last")

But if it is not important to make movement immediately I prefer to use Timer and ensure_main_thread to be sure that all callbacks will be processed first.

To be more accurate. The error in @brisvag code is caused by triggering a move before vispy layer was created and looking on @cnstt traceback it looks like vispy layer is created but the entry in layerlist is not added yet.

Czaki on Apr 26, 2022

Yes, this sounds familiar… I had a similar issue in the past, I think.

… Found it, #3337. Not sure if it’s related, but it might be.

I was able to reproduce some kind of error; it does not seem to be exactly the same as your message, however. Try this:

import napari
import numpy as np
v = napari.Viewer()

il = v.add_image(np.random.rand(10, 10), colormap='reds')

def move():
    v.layers.move(1, 0)

v.layers.events.inserted.connect(move)

il2 = v.add_image(np.random.rand(10, 10), colormap='blues')

maybe the event wizards @Czaki and @tlambert03 have some ideas 😃

brisvag on Apr 26, 2022