ome / ome-model

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

ome_model: do not use mutable default arguments #149

Closed sbesson closed 3 years ago

sbesson commented 3 years ago

Triggered by an IDR submission, the following code exposes the issue

from ome_model.experimental import Plate, Image, create_companion

for i in range(5):
    plate = Plate(str(i), 1, 1)
    well = plate.add_well(0,0)
    image = Image(
      str(i), 10, 10, 1, 2, 1,
                order="XYZCT", type="uint16")
    image.add_tiff(str(i) + '.tif')
    well.add_wellsample(0, image)
    create_companion(plates=[plate], out=str(i) + '.companion.ome')

While expecting this will create 4 companion OME files with 1 plate/1 well/1 well sample/1 image each, the second companion contains 2 images, the third 3...

Tracing down the source of the issue comes down to the usage of empty lists as default arguments which is a classical Python gotcha as these are mutable. At the moment, a workaround would be to explicitly pass a new empty list.

-    create_companion(plates=[plate], out=str(i) + '.companion.ome')
+    create_companion(plates=[plate], images=[], out=str(i) + '.companion.ome')

87f4859 updates all locations using empty lists as default to using None instead and handles the check within the method

All the other commits add some unit test coverage of the plate generation scenarios. The tests should be failing without 87f4859 and passing with.