8000 Fix cursor dimensionality race condition by jni · Pull Request #7295 · napari/napari · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix cursor dimensionality race condition #7295

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

Merged
merged 3 commits into from
Sep 26, 2024
Merged

Fix cursor dimensionality race condition #7295

merged 3 commits into from
Sep 26, 2024

Conversation

jni
Copy link
Member
@jni jni commented Sep 25, 2024

References and relevant issues

Closes #7248

Description

The cursor is supposed to have the same dimensionality as the full dims model.
However, #7248 exposed one of those situations where we want to simultaneously
update many objects, and the order winds up mattering: we try to update the
position of the cursor before the dimensionality of the cursor has been
updated, causing an IndexError.

In this PR, I simply check for the right dimensionality, and use a clear (all
zeros) position of the correct dimensionality if the cursor dimensionality is
out of date for the current slice request.

There's probably a better way to do this, but this fixes a rather severe bug
so I'd like to get this fix out quickly, and maybe refactor later.

The PR includes a new test to avoid a regression.

@jni jni requested a review from a team as a code owner September 25, 2024 08:19
@jni jni added this to the 0.5.4 milestone Sep 25, 2024
@github-actions github-actions bot added the tests Something related to our tests label Sep 25, 2024
@jni jni added bugfix PR with bugfix and removed tests Something related to our tests labels Sep 25, 2024
Copy link
codecov bot commented Sep 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.72%. Comparing base (d98c01b) to head (46a849e).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7295      +/-   ##
==========================================
- Coverage   92.81%   92.72%   -0.10%     
==========================================
  Files         623      623              
  Lines       57347    57354       +7     
==========================================
- Hits        53228    53182      -46     
- Misses       4119     4172      +53     

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

Copy link
Contributor
@DragaDoncila DragaDoncila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me!

@jni jni added the ready to merge Last chance for comments! Will be merged in ~24h label Sep 26, 2024
@jni jni merged commit 16d61e9 into napari:main Sep 26, 2024
48 checks passed
@jni jni deleted the neg-translate branch September 26, 2024 11:28
@github-actions github-actions bot removed the ready to merge Last chance for comments! Will be merged in ~24h label Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix PR with bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Napari crashes when add_image() called with negative z-axis translation and nD data
2 participants
0