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

Drop inserting in capture (changing in-place) and instead just add and then remove frames #233

Closed
wants to merge 1 commit into from

Conversation

psobolewskiPhD
Copy link
Member

Closes: #231

So in napari/napari#7150 event handling order was changed in napari.
This is making the key frame list manipulations for inserting a replacement frame not work correctly when the list change event callbacks trigger. I tried passing position='first' in the .connect() in various places and could not get it to resolve properly -- it's events of the SelectableEventedList, which are on the napari side I think.

I also tried to keep the name of the frame when insert=False, but the hash is still different so selection still fails.

In this PR I work around all this event stuff by removing the insert kwarg on capture_frame and no longer edit the frame in-place when replacing. Instead, the new frame is added and previous is removed.
Note that the frame number was already incrementing even when replacing a frame (insert=False) and because one can re-order frames, the numbers lose meaning.

Copy link

codecov bot commented Sep 5, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 86.08%. Comparing base (ba8ab53) to head (d1a07a0).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
napari_animation/_qt/animation_widget.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #233      +/-   ##
==========================================
- Coverage   86.24%   86.08%   -0.16%     
==========================================
  Files          26       26              
  Lines        1076     1071       -5     
==========================================
- Hits          928      922       -6     
- Misses        148      149       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@psobolewskiPhD psobolewskiPhD mentioned this pull request Sep 5, 2024
@psobolewskiPhD psobolewskiPhD changed the title Drop inserting in capture and instead just add and remove frames Drop inserting in capture (changing in-place) and instead just add and then remove frames Sep 5, 2024
@alisterburt
Copy link
Collaborator

Superseded by #234 - thanks for delving though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replacing keyframes breaks keyframe list
2 participants