Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

label properties #50

Merged
merged 15 commits into from
Nov 30, 2020
Merged

label properties #50

merged 15 commits into from
Nov 30, 2020

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Nov 18, 2020

Adds properties to labels .zattrs exported from OMERO.

See functionality tests below. But first question is probably what we want to name these properties and if we want them included by default or only with a cli argument?

        "properties": [
            {
                "label-value": 1,
                "omero:roiId": 212825,
                "omero:shapeId": 390575
            },

To Test - need an Image or Plate with masks

@will-moore will-moore changed the title Export omero:shapeId and omero:roiId in new label properties label properties Nov 22, 2020
Copy link
Member

@joshmoore joshmoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor question, otherwise, 👍

"omero:roiId": mask.roi.id.val,
}
if mask.textValue:
properties[mask_value]["text"] = unwrap(mask.textValue)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we assume that omero: is a "safe"-prefix for the moment, i.e. we can do what we want there, then probably the only question I have is the naming of the text field. @DragaDoncila proposed class in ome/ome-zarr-py#61. @will-moore, from your POV, would we post-process text into class where appropriate? Should it then be omero:text?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have to specify that this text came from OMERO in order to understand it. Similar to color - it might have come from OMERO, but we don't really care - we just want to know that the label has a color (or some text).
But these properties don't have defined keys in the spec (like flexible key-value pairs in OMERO) so I think we're "safe" to do what we want, without any prefix?
On the other hand I guess there's no harm to record the state that OMERO masks were in at the time of export, so I'm happy to use omero:text. I think class was just an example, rather than a reserved word?

@joshmoore
Copy link
Member

Otherwise, tested via:

$ omero hql --style=plain "select distinct s.textValue, s.roi.id from Shape s where s.roi.image.id = 5514374" --limit=-1 | tee rois.csv
$ omero zarr masks Image:5514374 --label-map=rois.csv
$ cat 5514374.zarr/labels/Chromosomes/.zattrs  | jq '.["image-label"]["properties"][0]'
{
  "label-value": 1,
  "omero:roiId": 1369051,
  "omero:shapeId": 1369067,
  "text": "Chromosomes"
}

@will-moore
Copy link
Member Author

That last commit is needed if we want to stitch together multiple Images + Labels, e.g. into a Plate as at ome/ome-zarr-py@4306bbb
and was used in my demo last week.

I noticed from that code, that (even before this change) we are only exporting 1 properties dict per ROI, not per shape. So if there are multiple Shapes per ROI, the omero:text value and omero:shapeId exported will simply be from the last one. This comes from the decision previously to have all 'shapes' exported with the same mask_value so that they are all displayed with the same color in napari (with 'auto' color mode). However, we could achieve the same thing by specifying a 'color' value in the labels metadata.

# Unused metadata: the{ZTC}, x, y, width, height, textValue
# Using ROI ID allows stitching label from multiple images
# into a Plate and not creating duplicates from different iamges.
# All shapes will be the same value (color) for each ROI
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds correct since we only support a UNION interpretation of shapes within a single ROI.

@joshmoore joshmoore self-assigned this Nov 26, 2020
@joshmoore
Copy link
Member

Ran successfully together with gh-53 after solving the merge conflict on Plate:2551. Waiting for labels to upload to visually verify.

@joshmoore joshmoore merged commit ae6cac0 into ome:master Nov 30, 2020
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.

3 participants