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

"labels" metadata section is confusing (and wrong?) #108

Open
constantinpape opened this issue Mar 15, 2022 · 4 comments
Open

"labels" metadata section is confusing (and wrong?) #108

constantinpape opened this issue Mar 15, 2022 · 4 comments

Comments

@constantinpape
Copy link
Contributor

constantinpape commented Mar 15, 2022

I am pretty confused by the "labels" metadata section:
It looks like this specifies the path to the first scale array. However, it would make much more sense to me if this specified the group with multiscales metadata for the label image.
I.e. instead of {"labels": ["orphaned/0"]} I would expect {"labels": ["orphaned"]}. Is this maybe just wrong?

On a sidenote: why do you call it orphaned? I think for labels something like mask or my-fancy-mask would make more sense as an example name.

@sbesson
Copy link
Member

sbesson commented Mar 16, 2022

I agree that the labels dictionary should point at the multiscales group rather than the first array of the group. Looking at the OME-NGFF samples generated by the OME team (see this blog post and [this blog posts[(https://www.openmicroscopy.org/2021/12/16/ome-ngff.html)) that contain labels, it looks like this is also how this was implemented.

(base) sbesson@Sebastiens-MacBook-Pro ~ % curl https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.3/idr0079A/9836998.zarr/labels/.zattrs
{
    "labels": [
        "0"
    ]
}%                                                                              (base) sbesson@Sebastiens-MacBook-Pro ~ % curl https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.3/idr0052A/5514375.zarr/labels/.zattrs
{
    "labels": [
        "Cell",
        "Chromosomes"
    ]
}%                                                                              (base) sbesson@Sebastiens-MacBook-Pro ~ % curl https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.1/6001247.zarr/labels/.zattrs
{
    "labels": [
        "masks"
    ]
}%  

I suspect this is an oversight and if so, it should be rectified at earliest. 👍 from my side for a PR updating the specification.
On the naming, I agree that orphaned leads to confusion. Maybe this should be unified with the label name used in the example layout for consistency?

@constantinpape
Copy link
Contributor Author

If I understand the example layout correctly this should be ["original/0"] because there is an intermediate empty group 0. This again is very confusing. I think we should avoid having these intermediate groups that are called like scale arrays or chunk hierarchys, it makes reading the whole spec much harder.

@sbesson
Copy link
Member

sbesson commented Mar 16, 2022

Also cross-linking to the concerns raised in #107 about the issue of using integer-named groups.

@will-moore
Copy link
Member

@constantinpape Yes, I think in the example layout, the /labels/.zattrs would contain that.
Actually, I noticed that the example does say
All labels will be listed in .zattrs e.g. { "labels": [ "original/0" ] }

But agreed, it would be simpler without the /0 extra level.

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

No branches or pull requests

3 participants