Fix bug with not display points close to given layer #2289
Conversation
napari/layers/points/points.py
Outdated
| distance = np.abs(data - indices[not_disp]) | ||
| matches = np.all(distance < 1e-5, axis=1) |
There was a problem hiding this comment.
I chose tolerance by random. I think that 10^(-5) ok, but I'm open.
There was a problem hiding this comment.
you could use https://numpy.org/doc/stable/reference/generated/numpy.allclose.html instead which has a default tolerance of 10^-8. Probably will be faster as i suspect they return as soon as they find something not close
There was a problem hiding this comment.
As i check allclose documentation thereis no axis argumen and allclose return single boolean operator.
I think that you mean isclose, but isclose need to allocate arrays of size equal to points, because here right value is single row.
This is why i use a manual way of comparision.
There was a problem hiding this comment.
ah ok, that's fair. not sure if @jni knows an alternative approach, otherwise your approach is fine
Codecov Report
@@ Coverage Diff @@
## master #2289 +/- ##
==========================================
+ Coverage 67.02% 67.32% +0.29%
==========================================
Files 402 408 +6
Lines 33760 34057 +297
==========================================
+ Hits 22629 22929 +300
+ Misses 11131 11128 -3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Change, looks good, I suggested using a builtin numpy function instead.
Can you look to see if we have a similar line of code in the shapes and vectors layer which should not use == too.
Also do you mind adding a test, then I think this will be good to go! Thanks for the quick fix
|
I think this implementation is fine for now. (eventually we'll want to do something more clever like cKDTree, but that's an existing problem.) @Czaki could you add a test with the failure that motivated this PR? |
I will do this tomorrow |
|
One strong disclaimer. n current napari implementation the slider step is calculated basing only on scale parameters. This causes that event with this PR point with z coordinate I see two solutions:
|
There was a problem hiding this comment.
Let's go with this now as it does fix a bug, but agreed, we likely still have more work to do to improve this. Thanks for the contribution @Czaki!
…dinates (#6629) # References and relevant issues This PR is created as result of debug #6627 (comment) As 0.4.19 branch has fewer problems than #6628 is created for 0.4.19. # Description Fix logic for selecting of visible shapes based on idea from #2289. It is lucky that we do not hit this earlier. Removes scaling of width of markers border (this is same like in #6628) To improve readability, this update `ShapesVisual` to have named subvisuals.
Description
Partially solve a problem described in #2288 which comes from float
==comparisonType of change
References
How has this been tested?