glencoesoftware / bioformats2raw

Bio-Formats image file format to raw format converter
GNU General Public License v2.0
82 stars 36 forks source link

OME/METADATA.ome.xml for Plate doesn't use #161

Closed will-moore closed 2 years ago

will-moore commented 2 years ago

Just reading https://github.com/ome/ngff/pull/112/files#diff-ffe6148e5d9f47acc4337bb319ed4503a810214933e51f5f3e46a227b10e3fcdR273 while looking at OME/METADATA.ome.xml generated for Plates at https://github.com/broadinstitute/lincs-cell-painting/issues/54 I noticed that the <Image /> elements are missing <MetadataOnly/> elements that are required by the proposed NGFF spec. I don't know if that OME/METADATA.ome.xml is valid or not. Downloaded sample with:

aws s3 sync s3://cellpainting-gallery/cpg0004-lincs/broad/images/2016_04_01_a549_48hr_batch1/images_zarr/SQ00014812__2016-05-23T20_44_31-Measurement1.ome.zarr/OME ./OME --no-sign-request
melissalinkert commented 2 years ago

MetadataOnly was added in https://github.com/glencoesoftware/bioformats2raw/pull/156, which is in 0.5.0-rc2. Which version was used to generate the sample data?

will-moore commented 2 years ago

Ah, thanks. According to https://github.com/broadinstitute/lincs-cell-painting/issues/54#issuecomment-1209465709 it looks like bioformats2raw 0.5.0rc1 was used. @sbesson do we care about missing <MetadataOnly/> elements? If we did, is there a way of re-generating the METADATA.ome.xml without doing a full conversion?

sbesson commented 2 years ago

I think the primary impact of missing <MetadataOnly/> is that the XML is invalid according to the OME schema. Possibly, the most immediate question is whether this would prevent the metadata population to be populated at import time or whether the reader can handle such XML.

If we need to correct the metadata without a full conversion, I think a script iterating through XML and adding a MetadataOnly element to each Image would be an option.

joshmoore commented 2 years ago

https://github.com/ome/ome-zarr-metadata/blob/main/src/ome_zarr_metadata/spec.py#L47 lest anyone start coding it up again.

melissalinkert commented 2 years ago

Closing since this is fixed and released in 0.5.0.