jni
commented
2 years ago This is looking very good @jamesyan-git! :heart_eyes: Looking forward to the ready-for-review!
Open jamesyan-git opened 2 years ago
jni
commented
2 years ago This is looking very good @jamesyan-git! :heart_eyes: Looking forward to the ready-for-review!
codecov-commenter
commented
2 years ago Merging #32 (345dddc) into main (0734529) will increase coverage by
6.37%. The diff coverage is100.00%.
:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more
@@ Coverage Diff @@
## main #32 +/- ##
==========================================
+ Coverage 78.11% 84.48% +6.37%
==========================================
Files 15 17 +2
Lines 635 896 +261
==========================================
+ Hits 496 757 +261
Misses 139 139 | Impacted Files | Coverage Δ | |
|---|---|---|
| src/zarpaint/_tests/test_watershed.py | 100.00% <ø> (ø) |
|
| src/zarpaint/__init__.py | 83.33% <100.00%> (+1.51%) |
:arrow_up: |
| src/zarpaint/_interpolate_labels.py | 100.00% <100.00%> (ø) |
|
| src/zarpaint/_tests/test_interpolate_labels.py | 100.00% <100.00%> (ø) |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
GenevieveBuckley
commented
2 years ago Does the user interaction here require one to push the plugin start button before painting the first slice?
I generally have use cases where partial labels might generated with some other plugin or separate software, and want to correct any areas of innacuracy. For this use case, it's a plus if you can select an already drawn label for the first and/or last slice (or touch up a tiny region at an edge, then use the whole label area for slice interpolation).
I don't quite know how much you plan to rely on a temp intermediate memory state for the painting, so I want to mention the above workflow so we can prioritize it too.
jamesyan-git
commented
2 years ago Hi @GenevieveBuckley , I totally understand about the use case regarding editing existing labels. I will aim to get this implemented once we are happy with the "from scratch" workflow. Below is a video showing the current workflow, where the following steps are taken:
Next steps:
I think this will put the annotate-from-scratch workflow in a good position and we can then start working on updating existing labels. What are your thoughts? I will keep adding docstrings and tests in the meantime.
GenevieveBuckley
commented
2 years ago I think that sounds like a reasonable plan
jamesyan-git
commented
1 year ago @DragaDoncila this is ready for review
jni
commented
1 year ago I just had a very quick look @jamesyan-git but it's looking good! I reckon add a couple of tests (interpolating 2D slices in 3D volume, and 3D in a 4D volume) and we merge!
GenevieveBuckley
commented
1 year ago It looks like there's been a change in the way napari handles events since this PR was first added.
When I try this out today with napari 0.4.16, clicking the slice interpolation button gives this error:
AttributeError: 'EmitterGroup' object has no attribute 'paint'related to line 202 in the enter_interpolation function:
self.selected_layer.events.paint.connect(self.paint_callback)
DragaDoncila
commented
1 year ago @GenevieveBuckley yeah the paint event hasn't actually been released yet, so this only works with main - and will soon work with 0.4.17 when it's released in the next few days
GenevieveBuckley
commented
1 year ago Had a quick look at this again and it functions well for 2d slices in a 3d volume, provided that the slice annotations overlap sufficiently (and that last part is a limitation of the algorithm we chose, as discussed previously).
I'm a lot less concerned that I was before about the usefulness of this in cases where a user needs to edit already generated labels. It turns out all you need to do is click "Start Interpolation", then click once with the paintbrush inside the already existing labels on the first and last slices, and then it will interpolate between them. So there's no need to worry about re-drawing edges correctly, which is the thing I was worried would be time consuming and difficult to do. 🎉
GenevieveBuckley
commented
1 year ago Here's a test I wrote annotating 2D slices in a 3D volume
from napari.layers import Labels
import numpy as np
from skimage.draw import ellipse
from zarpaint._interpolate_labels import interpolate_between_slices
label_id = 2
arraysize = 512
slice_index_1 = 0
slice_index_2 = 10
# First label slice
coords_1 = ellipse(arraysize//2, arraysize//2, # row, column (center coordinates)
arraysize//6, arraysize//6, # r_radius, c_radius (ellipse radii)
shape=(arraysize, arraysize))
label_slice_1 = np.zeros(shape).astype(int)
label_slice_1[coords_1] = label_id
# Second label slice
coords_2 = ellipse(arraysize//2, arraysize//2, # row, column (center coordinates)
arraysize//5, arraysize//3, # r_radius, c_radius (ellipse radii)
shape=(arraysize, arraysize))
label_slice_2 = np.zeros(shape).astype(int)
label_slice_2[coords_2] = label_id
# Create labels for interpolation
labels = np.zeros((11, arraysize, arraysize)).astype(int)
labels[slice_index_1] = label_slice_1
labels[slice_index_2] = label_slice_2
labels_layer = Labels(labels)
# Check all intermediate label slices contain only zero
for labels_slice in labels_layer.data[1:-1]:
assert np.max(labels_slice) == 0
# Check the expected number of non-zero pixels exist now
assert np.count_nonzero(labels_layer.data[1:-1]) == 0
# Interpolate intermediate slices
interpolate_between_slices(labels_layer, slice_index_1, slice_index_2, label_id, interp_dim=0)
# Check all intermediate label slices now contain non-zero pixels
for labels_slice in labels_layer.data[1:-1]:
assert np.max(labels_slice) == label_id
# Check the expected number of non-zero pixels exist now
assert np.count_nonzero(labels_layer.data[1:-1]) == 315045And here's a similar test, extended to a 4d array using ellispoid shapes. I've cut this down to interpolate across only three intermediate 3d volumes, so it runs in a reasonable amount of time (my %%timeit results: 1.6 s ± 37.8 ms per loop (mean ± std. dev. of 7 runs, 1 loop each))
from napari.layers import Labels
import numpy as np
from skimage.draw import ellipse
from zarpaint._interpolate_labels import interpolate_between_slices
label_id = 2
arraysize = 100
slice_index_1 = 0
slice_index_2 = 4
# First label slice
ellipse_1 = ellipsoid(20, 35, 25) * label_id
padding = np.array((arraysize, arraysize, arraysize)) - np.array(ellipse_1.shape)
pad_width = [(i//2, i//2 + 1) for i in padding]
label_slice_1 = np.pad(ellipse_1, pad_width)
# Second label slice
ellipse_2 = ellipsoid(28, 33, 40) * label_id
padding = np.array((arraysize, arraysize, arraysize)) - np.array(ellipse_2.shape)
pad_width = [(i//2, i//2 + 1) for i in padding]
label_slice_2 = np.pad(ellipse_2, pad_width)
# Create labels for interpolation
labels = np.zeros((5, arraysize, arraysize, arraysize)).astype(int)
labels[slice_index_1] = label_slice_1
labels[slice_index_2] = label_slice_2
labels_layer = Labels(labels)
# Check all intermediate label slices contain only zero
for labels_slice in labels_layer.data[1:-1]:
assert np.max(labels_slice) == 0
# Check the expected number of non-zero pixels exist now
assert np.count_nonzero(labels_layer.data[1:-1]) == 0
# Interpolate intermediate slices
interpolate_between_slices(labels_layer, slice_index_1, slice_index_2, label_id, interp_dim=0)
# Check all intermediate label slices now contain non-zero pixels
for labels_slice in labels_layer.data[1:-1]:
assert np.max(labels_slice) == label_id
# Check the expected number of non-zero pixels exist now
assert np.count_nonzero(labels_layer.data[1:-1]) == 297885
Wow, super cool @GenevieveBuckley, thanks for those tests! @jamesyan-git do you want to implement them and then we can merge this? 🎉
jamesyan-git
commented
1 year ago Look amazing, I will add this now. I'll also include some that I've been writing but forgot to push.
jni
commented
1 year ago @jamesyan-git it looks like there's some bits that are still not tested — check out the link for the "codecov/patch" failed check. It's things to do with the viewer widget itself. You can test them by calling viewer.window.add_plugin_dock_widget (from memory — there's a function like that somewhere in the hierarchy). Then the widget is returned and you can call particular methods in it.
jamesyan-git
commented
1 year ago Thanks Juan! I'm working on covering all the lines now.
jamesyan-git
commented
1 year ago hey @jni, I'm not sure why the macos tests are failing. any ideas?
GenevieveBuckley
commented
1 year ago hey @jni, I'm not sure why the macos tests are failing. any ideas?
Looking at the log, it looks like there was problem during Run GabrielBB/xvfb-action@v1
It seems that github action is deprecated (see here, and recommends switching to https://github.com/coactions/setup-xvfb. So give that a go, just try swapping it over in the gihutb actions workflow files and see what happens.
Allows users to draw labels on separate slices and interpolate in-between them Still need to improve UI, add tests, etc... Currently only works for first dimension