ome / ome-model

OME model (specification, code generator, implementation)
Other
13 stars 26 forks source link

Fix default z,c,t values of add_tiffdata #115

Closed sbesson closed 4 years ago

sbesson commented 4 years ago

Discovered while creating a minimal example of usage of this experimental library in https://forum.image.sc/t/spw-and-patterns/38625/5

If z,c,t are not passed or parsed from the TIFF files, the default value in theadd_tiff() is currently NOne and gets serialized as "None" in the resulting OME-XML which is obviously invalid.

Using a variation of the sample snippet copied in the image.sc issue, the following example can be used to test this PR

#!/usr/bin/env python
from ome_model.experimental import Plate, Image, create_companion

rows = 1
columns = 4
plate = Plate("test", rows, columns)
for row in range(rows):
    for column in range(columns):
        well = plate.add_well(row, column)
        basename = "%s%s" % (chr(row + 65), column)
        image = Image(basename, 512, 512, 1, 1, 1)
        image.add_tiff('%s_F1P0T0.ome.tiff' % basename)
        image.add_channel("channel", -1)
        well.add_wellsample(0, image)
create_companion(plates=[plate], out='plate.companion.ome')

Without this PR, the generated OME-XML shoudl contain multiple occurences <TiffData FirstC="None" FirstT="None" FirstZ="None"> while with this PR included, it should be <TiffData FirstC="0" FirstT="0" FirstZ="0"> as expected.

NB: the alternate implementation would be to skip the addition of the FirstZ/FirstC/FirstT attribute if z/c/t is None as these attributes are optional and default to 0 in the schema.

dgault commented 4 years ago

Setting to 0 makes sense here and None is certainly not valid.

Verified that without the PR the generated OME-XML has <TiffData FirstC="None" FirstT="None" FirstZ="None"> and with the PR it now contains <TiffData FirstC="0" FirstT="0" FirstZ="0"> as expected.

Looks good to merge from my side.