Adding load function to load ROIs from Omero to Napari

[MinaEnayat]

This PR introduces the load_rois(conn, image) function in src/napari-omero/plugins/loaders.py.

• Load ROIs and their shapes directly from OMERO for a given image
• Convert OMERO shapes (via_parse_omero_shape()_ like rectangles, polygons, and ellipses) into the correct Napari coordinate system
• Add all these processed shapes to a single Napari shapes layer. Crucially, it also carries over essential metadata such as the ROI ID, shape ID, Z/T index, and any comments, keeping all the relevant info accessible right there in Napari
• Apply physical pixel scaling and visual styling (color, edge width)

Issues:

• Currently, the implementation creates separate ROIs for each Z/T index when these values are none. I'm exploring how to implement functionality that would represent a single OMERO ROI spanning all Z/T planes in Napari (i.e., a single ROI covering all planes)
• Additionally, if I use the Upload Annotation feature after loading ROIs, it results in duplicate ROIs being saved in OMERO. I plan to address this in a future PR

Done under the guidance of @Tom-TBT

[psobolewskiPhD]

Wow this sounds awesome! I will try to test and review ASAP but i'm a bit slammed today and tomorrow 😞 Would be curious if this can round-trip with the save/upload implementation https://github.com/tlambert03/napari-omero/pull/100 ?

[codecov[bot]]

Codecov Report

:x: Patch coverage is 4.89510% with 136 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 34.31%. Comparing base (83ae41a) to head (1fdd064). :warning: Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/napari_omero/plugins/loaders.py	3.53%	109 Missing :warning:
src/napari_omero/widgets/ROIs.py	10.00%	27 Missing :warning:

:x: Your patch status has failed because the patch coverage (4.89%) is below the target coverage (85.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #116      +/-   ##
==========================================
- Coverage   36.21%   34.31%   -1.91%     
==========================================
  Files          14       14              
  Lines         947     1090     +143     
==========================================
+ Hits          343      374      +31     
- Misses        604      716     +112     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[MinaEnayat]

Hi @psobolewskiPhD,

Thanks for your response! I’ve already implemented the upload/save functionality in PR #100. At this point, I just want to confirm whether the current function/approach looks reasonable. I’d really appreciate your expert input to make sure I’m heading in the right direction — no rush at all. The remaining features will follow in upcoming PRs.

[psobolewskiPhD]

I pulled this down and it works really nicely -- for ROI only anyways. I'm unsure of the UI/widget part. I'm not sure I like using init to sort of hijack the existing widget? Does it make sense to have a unified annotation handling widget? At some point, it would be nice to load Points and Labels too, so we could already plan for such a future? Would like @jo-mueller to chime in!

Otherwise, I made very minor comments. I feel like some error/warnings could be nice, at least the case where nothing is returned so the user knows the function worked, but the roi were improper/not there.

[MinaEnayat]

Thanks for the feedback! I’ll add loading Points and Labels too. I see your point about the widget—maybe I’ll split it out into its own. I’ll add warnings where needed and fix the NumPy array instead of list as well.

[psobolewskiPhD]

I think Points and Labels could be followup PRs, up to you!

[jo-mueller]

Just came here to say I saw the PR, will give it a whirl tomorrow or the day after 👍 If you decide to start digging into Labels, make sure to have a look at this piece of code over at omero-cli-zarr. The conversion from OMERO ROIs to binary masks and labels is pretty much entirely written out already :)

[jo-mueller]

Delayed...having some issues with our omero server 🙄

[jo-mueller]

@psobolewskiPhD @MinaEnayat Finally got around to try this! Sorry it took me such a long time.

For me it generally works but I had some issues when trying it with the non-default group (security violation error - may be related to a different issue, though) - can somebody else reproduce this? Steps:

1. Log into default group - load ROI, works fine
2. Switch group, open different image, ROI cannot be loaded.

As for the question of whether or not to put it into the ROI widget, I guess it kind of makes sense to put it next to the upload button - maybe a sensible way to go could be to restructure the widget into a sort of ROIManager? For one thing, I like that the widget functionality retrieves the ROI based on the image id in the image's metadata, which seems the most future-proof way to go. I think it's also written in a way that makes it easily extensible for Points and Labels

Long story short - maybe rename the widget from Upload Annotations to OMERO into something like OMERO ROI Manager or OMERO ROI I/O or simply OMERO ROIs?

[Tom-TBT]

Hi @jo-mueller and @psobolewskiPhD first thank you very much for your insights on this PR. I'm not familiar much with napari so I'm glad that you did such a thorough review.

maybe a sensible way to go could be to restructure the widget into a sort of ROIManager

Agreed, also that we are thinking of more things to put in the ROI upload/download logic:

• upload all napari ROIs to OMERO (what it's doing now)
• update modified (or deleted) napari ROIs on OMERO (if ROIs were downloaded from OMERO)
• versioning ROIs with annotations (tags?) → I need to develop that feature in the OMERO.iviewer
• download subsets of ROIs (e.g. filter on tags, textValues)

That's quite a few things I want to have there so imo it's out of this PR scope. For this PR, I've told Tehmina to mainly look into the functionality aspect first, to solve the issues you pointed out. Once that's done, we'll clean up the UI for the two functionalities we will have, download all and upload all.

[jo-mueller]

@Tom-TBT thanks for chiming in! Just to clarify, from my side this PR could goo through with the minor changes suggested by Peter above. Renaming the ROI widget would alsoo suffice for me at this point of time, expanding it for comprehensive ROI handling is certainly work for future PRs. This one paves the way in the right direction, though 👍

PS: Tagging ROIs would be a 💯🚀feature! Let me know when you need beta testers 🤓

[Tom-TBT]

Sorry we had a little mix-up with the commits pushed yesterday. They are meant to be in a different branch for later (changes to save ROIs logic). Talking about commits 435c4ce to 8696a2. Current last one should be f77199e.

If you don't see them anymore when you read this, then all good.

[jo-mueller]

@MinaEnayat Thanks for the work - works like a charm on my end (tested the functionality from the ROI widget). One minor thing above, as per @psobolewskiPhD 's suggestion: I think it makes sense for the ROI download to use the image from the combobox in the widget rather than the selected image in the viewer's layer list.

In the future, the widget may yet become something like an ROI manager, but this PR shouldn't be held back by these considerations. That's more for us to decide where we'd want to go design-wise.

[jo-mueller]

Hi @MinaEnayat , we are merging #124 which contains your changes as well. Thanks for the contribution!