Wrapper improvements for ROIs

[Tom-TBT]

Changes in this PR are to increase the support of functionalities around ROIs:

• support annotations on ROIs (could be used to organize ROIs with tags, add properties with KV pairs, ...)
• getShapes for an ROI now retrieves wrapped shapes instead of ShapeI

[will-moore]

It is a nice improvement for roi.getShapes() to return ShapeWrapper instead of ShapeI but it is unfortunately a breaking API change.
Although roi.getShapes() isn't really documented or used in our examples, it is currently mapped to roi._obj._getShapes() and it's possible that users have code that would break with that change.

I noticed that _RoiWrapper has

    # TODO: test listChildren() to use ShapeWrapper? or remove?
    CHILD_WRAPPER_CLASS = 'ShapeWrapper'

And roi.listChildren() fails, since this expects a link class such as ProjectDatasetLink etc.
So you could rename your getShapes() to listChildren() and remove the CHILD_WRAPPER_CLASS = 'ShapeWrapper'.

To be consistent with other listChildren() behaviour, IF the shapes aren't loaded then they should be loaded on the fly (and probably cached) as we do in some other places.

[will-moore]

Could you also add support for loading Shapes on the fly in listChildren() if they're not already loaded?

[Tom-TBT]

Hey Will, sorry I'm taking this work back after a long pause. Working back on the tags & ROIs, I would need this to handle tags on ROIs.

Could you also add support for loading Shapes on the fly in listChildren() if they're not already loaded?

I reimplemented _listChildren and listChildren of RoiWrapper, so I don't need to hijack CHILD_WRAPPER_CLASS = 'ShapeWrapper', and so getChildLinks fails as it should.

These should now work:

roi_o = conn.getObject("Roi", 123)
shapes = list(roi_o.listChildren())
annotations = list(roi_o.listAnnotations())

print(shapes)
>> [<_ShapeWrapper id=1714>, <_ShapeWrapper id=1715>]
print(annotations)
>> [<TagAnnotationWrapper id=29>, <TagAnnotationWrapper id=28>]

[will-moore]

Was this change needed to fix something? I don't see that getAnnotationLinkTableName() is used anywhere now.

I've tried to make obj.listAnnotations() work for all object types over at #489, but I'd actually missed that getAnnotationLinkTableName() existed.

[will-moore]

Actually @Tom-TBT, could you remove the changes here and above for getAnnotationLinkTableName() as I've just updated that method to delegate to ann_link_name() in #489 - to cover all object types and this will now conflict with your changes.
Thanks

[Tom-TBT]

Thank you, I removed the changes. It works without it indeed.

[will-moore]

Thanks for getting back to this.
Not tested yet but I'm sure it's working...

I wonder if you could open a test PR to add a test like testGetROICount() at https://github.com/ome/openmicroscopy/blob/a49489e0cb2c30833afa413ae796f65478f6a387/components/tools/OmeroPy/test/integration/test_rois.py#L39 that does a similar setup etc then tests:

roi_o = conn.getObject("Roi", 123)
shapes = list(roi_o.listChildren())

Don't worry about testing roi annotations - I've got that covered in ome/openmicroscopy#6458 for #489

Thanks

[Tom-TBT]

I reverted my changes in getAnnotationLinkTableName. This was supposed to enable listing of Annotation from Roi, but is not needed anymore as #489 fixes it.

I also want to point out another change I have, to list Rois from an Annotation, in getParentLinks
Is it something that should stay here? Do you want to also have something here to not have to hardcode the list of class that can be annotated?