Closed thompson318 closed 5 months ago
Thank you for your pull request :smiley:
:robot: This automated message can help you check the rendered files in your submission for clarity. If you have any questions, please feel free to open an issue in {sandpaper}.
If you have files that automatically render output (e.g. R Markdown), then you should check for the following:
:mag: Inspect the changes: https://github.com/HealthBioscienceIDEAS/microscopy-novice/compare/md-outputs..md-outputs-PR-49
The following changes were observed in the rendered markdown documents:
config.yaml | 2 +
fig/binary_mask_erosion_1.png (new) | Bin 0 -> 165471 bytes
fig/binary_mask_erosion_10.png (new) | Bin 0 -> 134570 bytes
fig/binary_mask_erosion_5.png (new) | Bin 0 -> 151136 bytes
fig/binary_mask_no_erosion.png (new) | Bin 0 -> 168437 bytes
fig/install-region-props.png (new) | Bin 0 -> 92609 bytes
fig/instance_segmentation_clear_border.png (new) | Bin 0 -> 159902 bytes
fig/instance_segmentation_eroded.png (new) | Bin 0 -> 144598 bytes
fig/instance_segmentation_expanded.png (new) | Bin 0 -> 185378 bytes
...segmentation_vs_semantic_segmentation.png (new) | Bin 0 -> 128750 bytes
fig/instance_segmentation_wrong.png (new) | Bin 0 -> 273022 bytes
fig/instance_segmentation_wrong3d.png (new) | Bin 0 -> 696986 bytes
fig/region_props_after.png (new) | Bin 0 -> 330011 bytes
fig/region_props_after_18.png (new) | Bin 0 -> 337500 bytes
fig/region_props_after_3.png (new) | Bin 0 -> 333328 bytes
fig/region_props_before.png (new) | Bin 0 -> 288852 bytes
instance-segmentation-and-measurements.md (new) | 637 +++++++++++++++++++++
md5sum.txt | 3 +-
18 files changed, 641 insertions(+), 1 deletion(-)
:stopwatch: Updated at 2024-03-12 09:34:04 +0000
Thanks for the draft @thompson318 ! I like the general structure of moving from a semantic segmentation through to gradually better versions of the instance segmentation. I've added comments thoughout - mostly small things to fix.
Some bigger points to consider:
* It would be nice to still use some Napari plugins in this episode. Especially since the previous episode builds up to using `napari-segment-blobs-and-things...`, `napari-simpleitk...` and `napari-assistant`. I realise that erosion is not readily available through these plugins, but perhaps all segmentation steps apart from this could still be done via a plugin? (like gaussian, otsu, label, expand_labels...). I've also opened a PR on `napari-segment-blobs-and-things-with-membranes` to add erosion / dilation / opening / closing. If this gets merged soon, then all instance segmentation steps could be done through napari-assistant, if we wanted to * I've mentioned in the comments that [napari-script-editor](https://www.napari-hub.org/plugins/napari-script-editor) could be a good alternative to the Napari console for writing/saving larger code chunks. * I think it would still be nice to introduce some of the features of [napari-skimage-regionprops](https://www.napari-hub.org/plugins/napari-skimage-regionprops) to visualise size/shape measurements on images. This could always be an exercise, if you didn't want it as part of the main text. The plugin should already be installed as it's a dependency of `napari-simpleitk-image-processing`. It might be nice to show `Tools > Measurement tables > Regionprops (scikit-image, nsr)` with `size` and `shape` selected. Then you can double click on column headers to see a colour coded image of that measurement + also highlight individual labels and rows [as in their docs](https://www.napari-hub.org/plugins/napari-skimage-regionprops#usage-measure-region-properties)
Thanks @K-Meech , the script editor and region props plugins look good. I'll add them to the lesson. It's a shame the script editor doesn't have a nice output for print statements but we can probably work around that for now.
I'm not sure it makes sense to have some bits of the workflow done with plugins and the napari assistant and some bits done on the console. I can't find any way to integrate the two to make a single repeatable pipeline. Unless I'm mistaken we'd end up with one napari-assistant pipeline to get a semantic segmentation, then switch to the console, then back to a separate napari-assistant pipeline, which I think would be confusing. We'll have to discuss further.
No worries @thompson318 - let's stay with python scripts then, and not use the napari assistant. Perhaps just add a bit of context at the top of the lesson to show we're switching from plugins to scripts? For example, 'Last episode, we used a series of napari plugins to produce a semantic segmentation of our nuclei. In this episode, we'll continue working with the same image, exploring how python scripts can be used to make reproducible workflows....'
@thompson318 and @K-Meech Are you ready for me to take a look at this one?
@thompson318 and @K-Meech Are you ready for me to take a look at this one?
@davecash75 I'm happy for you to take a look if you have time. I'm working through it today and tomorrow to address @K-Meech comments but happy for you to add to them. The main issue is that I've moved away from using plugins and napari assistant because I wanted the students to be able to create a complete pipeline to go from image to measurement. At present there are bits missing from the plugin/assistant setup that stop that from being done.
No worries @thompson318 - let's stay with python scripts then, and not use the napari assistant. Perhaps just add a bit of context at the top of the lesson to show we're switching from plugins to scripts? For example, 'Last episode, we used a series of napari plugins to produce a semantic segmentation of our nuclei. In this episode, we'll continue working with the same image, exploring how python scripts can be used to make reproducible workflows....'
Added a note at start on why we're using python console c54ca64
This is ready for second review. I've left some comments unresolved as I'm not certain that they're the best solution yet.
Also, I had some missing links to images that were not in my repository? Did I somehow miss them and they should be included?
Also, I had some missing links to images that were not in my repository? Did I somehow miss them and they should be included?
That should be fixed now, sorry. I just forgot to commit the images.
@davecash75 @K-Meech I think this is all good now. If you could please take a quick look and approve if you're happy I will merge.
Thanks @thompson318 - but I think there are still more of Dave's comments to address, if I'm not mistaken? E.g. https://github.com/HealthBioscienceIDEAS/microscopy-novice/pull/49#discussion_r1509750521, https://github.com/HealthBioscienceIDEAS/microscopy-novice/pull/49#discussion_r1509746688 and others... They still show as un-resolved above?
Thanks @thompson318 - but I think there are still more of Dave's comments to address, if I'm not mistaken? E.g. #49 (comment), #49 (comment) and others... They still show as un-resolved above?
Sorry, not sure how I missed them. There's a lot of comments on this, I think they just got squashed. I'll sort them first.
@K-Meech and @davecash75 I think I've got them all resolved this time. I've left 3 unresolved as they were not straightforward and just wanted to get @K-Meech opinion on whether they are successfully resolved.
Thanks @thompson318 - this is looking great! Really like the new regionprops section + slower introduction of python principles like for loops.
I've added some comments, they're all very minor things like typos + areas where the line length is too long.
Thanks @K-Meech for picking up the errors. I've committed your changes now.
Draft of episode 6 on instance segmentation and measurement.