ome / omero-py

Python project containing Ice remoting code for OMERO
https://www.openmicroscopy.org/omero
GNU General Public License v2.0
20 stars 31 forks source link

add getROIs #380

Closed barrettMCW closed 10 months ago

barrettMCW commented 11 months ago

hey y'all! I was going to put a patch into our groups dev library but i figured I'd see if you wanted it. Since getROICount already gets the ROIs i just put the get part in a different method and made count just check the length of results.

will-moore commented 11 months ago

Hi, and thanks for the suggestion.

Methods of the BlitzGateway (e.g. getROIs()) would normally be expected to return BlitzObjectWrapper objects, not e.g. omero.model.Roi but rather wrap the roi with omero.gateway.RoiWrapper(conn, roi).

However, you wouldn't want to init all those objects when just counting ROIs as there's potential for BlitzObjectWrapper to load more data on init. So you might want a def _get_rois() to return omero.model.roi objects as you currently have for getROIs(), use this for getROICount() and then wrap those ROIs to return Wrapper objects for def getROIs(). Does that make sense?

barrettMCW commented 11 months ago

Good stuff, thanks! getROIs and getROICount now use a private function _get_rois to get valid roi model objects, and getROIs wraps those and returns them.

will-moore commented 10 months ago

Just added the --include label, since I noticed this wasn't being merged in the daily build: see https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-python-superbuild-push/271/console

Then we can confirm tests are still passing at https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-test-integration/lastCompletedBuild/testReport/OmeroPy.test.integration.test_rois/

In due course, it would also be nice to add tests for conn.getROIs() to https://github.com/ome/openmicroscopy/blob/develop/components/tools/OmeroPy/test/integration/test_rois.py

will-moore commented 10 months ago

PR is being merged now, e.g. https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-python-superbuild-push/273/console and corresponding daily tests are passing https://merge-ci.openmicroscopy.org/jenkins/job/OMERO-test-integration/lastCompletedBuild/testReport/OmeroPy.test.integration.test_rois/