-
Notifications
You must be signed in to change notification settings - Fork 8
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
[WIP] Add slice interpolation #32
base: main
Are you sure you want to change the base?
Conversation
This is looking very good @jamesyan-git! 😍 Looking forward to the ready-for-review! |
src/zarpaint/_interpolate_labels.py
Outdated
from scipy.ndimage import distance_transform_edt | ||
|
||
|
||
def distance_transform(image): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imho this should be renamed signed_distance_transform
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamesyan-git can you rename this function? 😉
Codecov Report
📣 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
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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. |
Hi @GenevieveBuckley ,
@jni 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. |
I think that sounds like a reasonable plan |
@DragaDoncila this is ready for review |
@@ -42,3 +45,5 @@ contributions: | |||
display_name: Split With Watershed | |||
- command: zarpaint.copy_data | |||
display_name: Copy Data | |||
- command: zarpaint.interpolate_widg | |||
display_name: Interpolate Slices |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
display_name: Interpolate Slices | |
display_name: Interpolate Slices with Signed Distance Transform |
Just preparing for when we add future methods. I also hate that it's so hard for me to find out in other interpolation tools what method they're actually using!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like when we have future methods, there should be a dropdown selection box in the interpolation widget to switch between them (instead of creating additional widgets, which this comment seems to suggest).
@@ -23,6 +23,9 @@ contributions: | |||
- id: zarpaint.copy_data | |||
title: Copy Data… | |||
python_name: zarpaint:copy_data | |||
- id: zarpaint.interpolate_widg | |||
title: Interpolate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
title: Interpolate | |
title: Interpolate Slices with Signed Distance Transform |
?
Maybe we can remove "signed" or Interpolate Slices (w/ Distance Transform)
if length is an issue
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! |
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:
related to line 202 in the
Error message:Traceback (most recent call last):
File "/Users/genevieb/mambaforge/envs/napari-empanada/lib/python3.9/site-packages/magicgui/widgets/_bases/value_widget.py", line 57, in _on_value_change
self.changed.emit(value)
File "psygnal/_signal.py", line 725, in psygnal._signal.SignalInstance.emit
File "psygnal/_signal.py", line 767, in psygnal._signal.SignalInstance._run_emit_loop
File "psygnal/_signal.py", line 768, in psygnal._signal.SignalInstance._run_emit_loop
File "psygnal/_signal.py", line 788, in psygnal._signal.SignalInstance._run_emit_loop
File "/Users/genevieb/Documents/GitHub/zarpaint/src/zarpaint/_interpolate_labels.py", line 252, in enter_interpolation
self.selected_layer.events.paint.connect(self.paint_callback)
File "/Users/genevieb/mambaforge/envs/napari-empanada/lib/python3.9/site-packages/napari/utils/_proxies.py", line 72, in __getattr__
return self.create(super().__getattr__(name))
File "/Users/genevieb/mambaforge/envs/napari-empanada/lib/python3.9/site-packages/napari/utils/events/event.py", line 971, in __getattr__
return object.__getattribute__(self, name)
AttributeError: 'EmitterGroup' object has no attribute 'paint' napari.sys_info():napari: 0.4.16
Platform: macOS-12.3.1-arm64-arm-64bit
System: MacOS 12.3.1
Python: 3.10.6 | packaged by conda-forge | (main, Aug 22 2022, 20:41:22) [Clang 13.0.1 ]
Qt: 5.15.6
PyQt5: 5.15.7
NumPy: 1.23.4
SciPy: 1.9.3
Dask: 2022.10.2
VisPy: 0.10.0
OpenGL:
- GL version: 2.1 Metal - 76.3
- MAX_TEXTURE_SIZE: 16384
Screens:
- screen 1: resolution 1440x900, scale 2.0
- screen 2: resolution 1920x1080, scale 1.0
Plugins:
- console: 0.0.6
- napari-svg: 0.1.6
- scikit-image: 0.4.16
- zarpaint: 0.1.1.dev27+gfc48307 conda list:
|
@GenevieveBuckley yeah the |
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. 🎉 |
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]) == 315045 |
And 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 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? 🎉 |
Look amazing, I will add this now. I'll also include some that I've been writing but forgot to push. |
@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 |
Thanks Juan! I'm working on covering all the lines now. |
src/zarpaint/_interpolate_labels.py
Outdated
|
||
if slice_index_1 > slice_index_2: | ||
slice_index_1, slice_index_2 = slice_index_2, slice_index_1 | ||
layer_data = np.asarray(label_layer.data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line needs to be removed. (I assume it's here because you were debugging something, but we can't have an entire full size volume read into memory).
We should replace the following two lines with:
slice_1 = np.take(label_layer.data, slice_index_1, axis=interp_dim)
slice_2 = np.take(label_layer.data, slice_index_2, axis=interp_dim)
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 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. |
…t into slice-interpolation
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