Synced slider with frame index rather than active keyframe

[Fifourche]

A PR to link the AnimationSlider with the frame index being set.

Before it was synced with the change of key_frames.selection.active, and now with the signal of an Emitter : frame_index, added to Animation. This way, if we use set_movie_frame_index, it'll put the slider at the frame's position and not at the closest key frame.

For some context, the idea I had was to sync the slider with a frame_index signal to prepare the way for a "previewer" capability. The preview would be launched via a method of Animation, or a button next to the slider.

[codecov[bot]]

Codecov Report

Merging #99 (643e5fc) into main (aab5909) will increase coverage by 0.51%. The diff coverage is 90.38%.

@@            Coverage Diff             @@
##             main      #99      +/-   ##
==========================================
+ Coverage   90.08%   90.59%   +0.51%     
==========================================
  Files          20       20              
  Lines         827      851      +24     
==========================================
+ Hits          745      771      +26     
+ Misses         82       80       -2     

Impacted Files	Coverage Δ
napari_animation/animation.py	86.17% <83.33%> (+7.34%)	:arrow_up:
napari_animation/_qt/animation_widget.py	85.55% <88.88%> (-2.36%)	:arrow_down:
napari_animation/frame_sequence.py	98.83% <95.83%> (-1.17%)	:arrow_down:
napari_animation/_tests/test_frame_sequence.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update aab5909...643e5fc. Read the comment docs.

[alisterburt]

Okay cool! Maybe we could refactor this logic into a ‘has_frames’ properly rather than adding it here?

[Fifourche]

Wasn't sure of what you meant, as it's mostly the number of key frames that is needed there. Would you still go for an attribute for this ?

Okay cool! Maybe we could refactor this logic into a ‘has_frames’ properly rather than adding it here? … On 1 Jun 2021, at 12:19, Fifourche @.***> wrote:  @Fifourche commented on this pull request. In napari_animation/frame_sequence.py: > self.events.n_frames(value=len(self)) return - f = 0 - for kf0, kf1 in pairwise(self._key_frames): - for s in range(kf1.steps): - fraction = s / kf1.steps - self._frame_index[f] = (kf0, kf1, fraction) - f += 1 + else: + f = 0 + if len(self._key_frames) == 1: + kf1 = self._key_frames[0] Previously, if there was only one keyframe, it was not added as a frame and _frames was empty. I just added this here so _frames would be empty only when there are no keyframes at all. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

[Fifourche]

Hey :) I'm changing a bit this PR to switch the current frame index attribute from animation to FrameSequence ! In my eyes it makes FrameSequence a bit more like an virtual slider, which would be relevant to connect to the GUI slider, and thus prepare the ground for a play button with #100 .