napari / napari-animation

A napari plugin for making animations
https://napari.github.io/napari-animation/
Other
79 stars 27 forks source link

Replacing keyframes breaks keyframe list #231

Closed psobolewskiPhD closed 2 months ago

psobolewskiPhD commented 2 months ago

Tests are failing on CI: https://github.com/napari/napari-animation/actions/runs/10724068457/job/29738857108?pr=230#step:8:123

Can reproduce locally with:

pytest napari_animation/_qt/_tests/test_animation_widget.py

Bisect points to this napari commit: 3c7d1a463df65e0e3b9668d4647cb96d066cda73 https://github.com/napari/napari/pull/7150

Edit: So this isn't just a test fail. I can reproduce this locally using the plugin. If you attempt to replace a keyframe (alt-r) it will:

``` --------------------------------------------------------------------------- ValueError Traceback (most recent call last) File ~/Documents/dev/napari/napari/_qt/containers/_base_item_view.py:78, in _BaseEventedItemView.selectionChanged(self=, selected=, deselected=) 75 desel = {i.data(ItemRole) for i in deselected.indexes()} 77 if not self._root.selection.events.changed._emitting: ---> 78 self._root.selection.update(sel) sel = {} self._root = [] self = 79 self._root.selection.difference_update(desel) 80 return super().selectionChanged(selected, deselected) File ~/Documents/dev/napari/napari/utils/events/containers/_set.py:118, in EventedSet.update(self=Selection(set()), others={}) 116 to_add = {self._pre_add_hook(i) for i in to_add} 117 self._set.update(to_add) --> 118 self._emit_change(added=set(to_add), removed=set()) to_add = {} self = Selection(set()) File ~/Documents/dev/napari/napari/utils/events/containers/_selection.py:80, in Selection._emit_change(self=Selection(set()), added={}, removed=set()) 78 if removed is None: 79 removed = set() ---> 80 self._update_active() self = Selection(set()) 81 return super()._emit_change(added=added, removed=removed) File ~/Documents/dev/napari/napari/utils/events/containers/_selection.py:127, in Selection._update_active(self=Selection(set())) 122 """On a selection event, update the active item based on selection. 123 124 (An active item is a single selected item). 125 """ 126 if len(self) == 1: --> 127 self.active = next(iter(self)) self = Selection(set()) 128 elif self._active is not None: 129 self._active = None File ~/Documents/dev/napari/napari/utils/events/containers/_selection.py:119, in Selection.active(self=Selection(set()), value=) 117 self.clear() if value is None else self.select_only(value) 118 self._current = value --> 119 self.events.active(value=value) value = self = Selection(set()) self.events.active = self.events = File ~/Documents/dev/napari/napari/utils/events/event.py:764, in EventEmitter.__call__(self=, *args=(), **kwargs={'value': }) 761 self._block_counter.update([cb]) 762 continue --> 764 self._invoke_callback(cb, event if pass_event else None) event = self = cb = > pass_event = True 765 if event.blocked: 766 break File ~/Documents/dev/napari/napari/utils/events/event.py:802, in EventEmitter._invoke_callback(self=, cb=>, event=) 800 self.disconnect(cb) 801 return --> 802 _handle_exception( self = event = cb = > (cb, event) = (>, ) 803 self.ignore_callback_errors, 804 self.print_callback_errors, 805 self, 806 cb_event=(cb, event), 807 ) File ~/Documents/dev/napari/napari/utils/events/event.py:789, in EventEmitter._invoke_callback(self=, cb=>, event=) 787 try: 788 if event is not None: --> 789 cb(event) event = cb = > 790 else: 791 cb() File ~/Documents/dev/napari-animation/napari_animation/animation.py:268, in Animation._on_active_keyframe_changed(self=, event=) 266 if active_keyframe: 267 keyframe_index = self.key_frames.index(active_keyframe) --> 268 self.set_key_frame_index(keyframe_index) keyframe_index = 0 self = File ~/Documents/dev/napari-animation/napari_animation/animation.py:120, in Animation.set_key_frame_index(self=, index=0) 118 def set_key_frame_index(self, index: int): 119 frame_index = self._keyframe_frame_index(index) --> 120 self.set_movie_frame_index(frame_index) frame_index = 0 self = File ~/Documents/dev/napari-animation/napari_animation/animation.py:132, in Animation.set_movie_frame_index(self=, index=0) 130 # to prevent active callback again 131 if self.key_frames.selection.active != key_frame: --> 132 self.key_frames.selection.active = key_frame key_frame = self = self.key_frames = [] 134 self._frames.set_movie_frame_index(self.viewer, index) 135 self._current_frame = index File ~/Documents/dev/napari/napari/utils/events/containers/_selection.py:117, in Selection.active(self=Selection(set()), value=) 115 return 116 self._active = value --> 117 self.clear() if value is None else self.select_only(value) value = self = Selection(set()) value is None = False 118 self._current = value 119 self.events.active(value=value) File ~/Documents/dev/napari/napari/utils/events/containers/_selection.py:145, in Selection.select_only(self=Selection(set()), obj=) 143 """Unselect everything but `obj`. Add to selection if not present.""" 144 self.intersection_update({obj}) --> 145 self.add(obj) obj = self = Selection(set()) File ~/Documents/dev/napari/napari/utils/events/containers/_set.py:79, in EventedSet.add(self=Selection(set()), value=) 77 """Add an element to the set, if not already present.""" 78 if value not in self: ---> 79 value = self._pre_add_hook(value) value = self = Selection(set()) self._pre_add_hook = ]> 80 self._set.add(value) 81 self._emit_change(added={value}, removed=set()) File ~/Documents/dev/napari/napari/utils/events/containers/_selectable_list.py:55, in SelectableEventedList._preselect_hook(self=[], value=) 53 """Called before adding an item to the selection.""" 54 if value not in self: ---> 55 raise ValueError( trans = value = 56 trans._( 57 'Cannot select item that is not in list: {value!r}', 58 deferred=True, 59 value=value, 60 ) 61 ) 62 return value ValueError: Cannot select item that is not in list: ```

The new keyframe cannot be removed and gives the traceback every time you try to click on it.

switching to '3c7d1a463df65e0e3b9668d4647cb96d066cda73~1'

psobolewskiPhD commented 2 months ago

I just checked locally, reverting the change makes tests pass again. Seems like this was something @brisvag had been concerned about: https://github.com/napari/napari/pull/7150/files#r1701596581 cc: @Czaki