-
Notifications
You must be signed in to change notification settings - Fork 12
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
Revamp color metadata #62
Comments
(Basically, from the point of view of developing in Java, it's generally not fun to try to interpret the Zarr v2 spec. 😕 Compressors are another thorny bit.) |
I'd have thought we'd want to stick to using JSON types where possible, so native booleans rather than strings representing booleans. |
I was surprised by that too. I'm adapting it from https://github.com/ome/omero-ms-zarr/blob/6add6804/spec.md#user-content-color, don't know why it needs stringifying though. |
Is it an opaque string, and the fact it happens to be a number is irrelevant? i.e. each object has a string label that is associated with a color? |
I think the image's pixel values have to fit the |
Must be because JSON only allows string keys |
Though I'd argue Perhaps this should be an array where array-index = object id, and if an object is missing it's set to |
Aha, that'll be it, thank you. Could be we decide to opt for 1/0 for split masks, it's an easy change to make to the microservice. I wonder if that violates what Zarr readers expect for |
cf:
@manics: can you outline the trade-offs between the two representations. The first proposal (#65) is to have the colors in an array the size of the labels, right?
with the downside that choosing large label values leads to large arrays. (One option would be to put this array in zarr rather than json?) The other proposal (#68) is basically to make the color metadata more explicit, right?
and then allowing to potentially update the key in the object to be something other than "rgba", e.g. "hex" or whatever? |
Yes, pretty much what you said. The other benefit of the second option is it's extensible to include other metadata about a label, for instance you could add a name/description to each label. In this case it might be better to rename I think we should also change the current default colour model from "an int in whatever format OMERO uses" to a more understandable 4-tuple: |
Decisions from todays Zoom call:Agreed:
Rejected:
To be decided:Keep the existing layout:"color": {
"1": [255, 255, 255, 0],
"4": [0, 255, 255, 128],
...
} Other metadata will be stored in a different top-level key, but with the same structure. Sparse list of dicts:"label-metadata": [
{
"label-value": 1,
"rgba": [255, 255, 255, 0]
},
{
"label-value": 4,
"rgba": [0, 255, 255, 128]
},
...
] Sparse dict of dicts where key is the label-value converted to a string:"label-metadata": {
"1": {
"rgba": [255, 255, 255, 0]
},
"4": {
"rgba": [0, 255, 255, 128]
},
...
} |
I prefer the first of the 3 options (most concise), but I guess the complexity of the labels parsing we're doing now for ome-zarr-py won't be very much different for any of these alternatives: |
Just noticed a bug in https://github.com/ome/ome-zarr-py/blob/208952c770c5d3683a8a8339173c0bc6ec158e54/ome_zarr.py#L280-L281 |
Fixed in ome/ome-zarr-py@db43af5 |
If I may add my two cents. sparse list of dicts: it is very explicit, support for a different color model is trivial, label names can be different types than strings. But the question is whether this freedom is desired? If not, than the current layout is probably fine. What I like about the other two approaches is that they are more human readable. I understand them without reading the specs. Wouldn't be so sure about the current layout. |
Currently working on a set of PRs to test out the following
|
Reading through https://qupath.readthedocs.io/en/latest/docs/advanced/exporting_annotations.html#binary-labeled-images, one wonders if the |
Separately, also considering an array (rather than JSON metadata) similar to this example:
|
This issue has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/multi-scale-image-labels-v0-1/43483/1 |
Split masks
For split masks the microservice specifies
"dtype":"|b1"
and the Zarr spec defines a Boolean data type as,and describes how metadata is encoded using JSON. However, in JSON, Boolean types are all-lowercase, no initial capital letter as above.
The microservice takes the JSON Boolean idea in offering, e.g.,
"fill_value":false
and"color":{"true":-2139062144}
. However, it's possible that it's meant to do a more-Pythonic initial-capital thing, or just use integers? (With 0 as false and not-0 as true? #60 (comment) suggests 1.)Such changes are trivial, it's just very unclear. If anybody arrives at certainty, happy to open a fixing PR.
The text was updated successfully, but these errors were encountered: