Prototype napari plugin widget for annotator_2d #177
Conversation
There was a problem hiding this comment.
Thanks for starting to work on this @GenevieveBuckley! I checked the high level structure and this perfectly matches what we discussed in #167. I haven't tried running it myself yet, since I am not sure if it's ready for that yet. Let me know if/once it is and then I will test it!
Besides that I just left a few minor comments with clarifications (and one minor change request).
Let me know if you need any input from my side to continue!
| return v | ||
|
|
||
|
|
||
| def _update_viewer(v, raw, show_embeddings, segmentation_result): |
There was a problem hiding this comment.
This function is needed by the image_series_annotator. I understand why you removed it (the current logic will not work any longer and we need to change it when adapting the image_series_annotator), but please leave in this function for now (without calling it), so that we have it as a reference for the series annotator.
| model_type: AVAILABLE_MODELS = AVAILABLE_MODELS[_DEFAULT_MODEL], | ||
| tile_shape: Tuple[int, int] = (None, None), | ||
| halo: Tuple[int, int] = (None, None), | ||
| predictor: Optional[SamPredictor] = None, # not accessible via magicgui widget, not a recognised type |
There was a problem hiding this comment.
Regarding the comment: that's ok. This doesn't make sense to pass via napari anyways. The predictor is currently exposed for 2 reasons:
- To pass the already initialized predictor in the
image_series_annotator, where the logic for image embeddings is done externally. - To enable passing predictors that were loaded from a custom checkpoint.
As far as I understand your comment, it doesn't hurt to leave it in for now. After we have merged this PR I can rethink a bit how this is handled because: re 1. we need to change the image_series_annotator logic anyways and re 2.: it would be good to enable loading of custom models also from the napari plugin. (Here: custom model means that we are using a url or filepath to specifiy the checkpoint rather than one of the names from AVAILABLE_MODELS).
| util.set_precomputed(PREDICTOR, IMAGE_EMBEDDINGS) | ||
|
|
||
| # viewer is freshly initialized | ||
| if v is None: |
There was a problem hiding this comment.
Just fyi: this condition is for the image_series_annotator: in that case we pass the napari viewer object via the v argument. Hence it is not None and _update_viewer is called to replace all relevant layers.
I just wanted to clarify because it relates to my comment on _update_viewer. We can go ahead and remove the condition since the image_series_annotator needs to be redone after finalizing the 2d annotator.
|
Hi @GenevieveBuckley , For this PR I suggest you only focus on the 2d annotation plugin, i.e. that
|
|
Superseeded by recent changes. |
Relates to #167