Update point add to handle multiple coordinates in data_indices #7536
Conversation
…rdinates in data_indices
for more information, see https://pre-commit.ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7536 +/- ##
==========================================
- Coverage 92.88% 92.84% -0.05%
==========================================
Files 632 632
Lines 59258 59258
==========================================
- Hits 55042 55018 -24
- Misses 4216 4240 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Hey @ClementCaporal, thanks for the PR! I would almost call this a bugfix, since the previous behaviour was just kinda wrong... But it's true that it changes the api, so maybe both bugfix and breaking change :P the release after 0.5.6 will probably be 0.6, so we could put this in there, if we're worried about breaking worklfows.
I have a suggestion: better to use np.arange for better performance, in case the user passes a large number of points (this returns an array instead of a tuple, but I think it's better here?)
In [11]: %timeit tuple(-i - 1 for i in reversed(range(len(coords))))
1.37 μs ± 11.5 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
In [12]: %timeit np.arange(-len(coords), 0)
875 ns ± 12.7 ns per loop (mean ± std. dev. of 7 runs, 1,000,000 loops each)
That's a very good question! And I don't know the answer to it!!! 😬 I would say leave it alone — actually even -1 is misleading... |
|
Thank you for your reviews @brisvag @jni
|
References and relevant issues
Closes #7507
Description
Two interrogations:
I couldn't check this final checklist: I feel the API changed, but I didn't find the appropriate docstring
.. versionadded::or.. versionchanged::directive to the appropriate docstring (For more information see
the Sphinx documentation).
Should event ActionType.ADDING have all the indices even if they haven't been added yet? here :
napari/napari/layers/points/points.py
Line 2115 in 194cca1