Skip to content

Wrapper improvements for ROIs#459

Draft
Tom-TBT wants to merge 4 commits into
ome:masterfrom
Tom-TBT:extend_annotations
Draft

Wrapper improvements for ROIs#459
Tom-TBT wants to merge 4 commits into
ome:masterfrom
Tom-TBT:extend_annotations

Conversation

@Tom-TBT
Copy link
Copy Markdown
Contributor

@Tom-TBT Tom-TBT commented May 2, 2025

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
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants