Add support for passing wells as List[dict] in write_plate_metadata

[sbesson]

Follow-up of #153, this adds support for wells specified as lists of dictionaries rather than lists of string. The primary use case are:

1- support additional custom metadata e.g.

wells = [
    {"path": "A/1", "concentration": "1"},
    {"path": "A/2", "concentration": "2"},
    {"path": "B/1", "concentration": "5"},
]
write_plate_metadata(self.root, ["A", "B"], ["1", "2"], wells)

2- support the upcoming breaking changes proposed in ome/ngff#24 i.e.

wells = [
    {"path": "A/1", "rowIndex": 0, "columnIndex": 0},
    {"path": "A/2", "rowIndex": 0, "columnIndex": 1},
    {"path": "B/1", "rowIndex": 1, "columnIndex": 0},
]
write_plate_metadata(self.root, ["A", "B"], ["1", "2"], wells)

Unit tests are updated accordingly to cover all variants. While working on the tests, I found that the usage of the list mutability by the _validate* API was creating unnecessary confusion when the input argument is re-used. 347da42 updates the logic to create a copy of the input list when the content might be modified by the validation function.

Since this API will likely be released with 0.4 support and given the changes proposed in ome/ngff#24, it is likely the former support for wells as List[str] will:
1- either need to be removed as a list of paths will not be sufficient to generate valid wells
2- or be updated to include the assumptions that the wells are following the standard letter/number convention and auto-generate the underlying rowIndex/columnIndex indices.
I left this discussion outside the scope of this PR which only focuses on supporting the List[dict] form.

[codecov]

Codecov Report

Merging #157 (9f065af) into master (2ed4426) will increase coverage by 0.63%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #157      +/-   ##
==========================================
+ Coverage   80.49%   81.12%   +0.63%     
==========================================
  Files          11       11              
  Lines        1174     1192      +18     
==========================================
+ Hits          945      967      +22     
+ Misses        229      225       -4     

Impacted Files	Coverage Δ
ome_zarr/writer.py	97.14% <100.00%> (+3.70%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ed4426...9f065af. Read the comment docs.

[joshmoore]

Barring the same discussion around possible prefixes for unparsed keys, 👍

[will-moore]

Looks good.
With the breaking spec change, I think the API support for list of strings should be removed, since the logic for guessing the correct row/col indices will be tricky and not easy to document well so users will understand (potential for getting it wrong).

[sbesson]

Re list of strings, my consideration was to limit the scope of supporting conventions and only consider the single standard scenario where row == letter, column == 1-based index i.e. support wells = ["A/3", "B/2", "B/5"] and compute the indices. Probably the biggest decision in that case would be to agree on what happens if len(rows) > 26 - see https://www.openmicroscopy.org/Schemas/Documentation/Generated/OME-2016-06/ome_xsd.html#Plate_RowNamingConvention.

But I am okay with leaving it the caller responsibility to generate the row/column indices first if we believe this is unnecessary. We can always add convenience APIs later

[will-moore]

I guess that is kinda convenient. And we will also have the rows and columns names, so we could simply split on / and see if they match:

r, c = well.split("/")
rowIndex = rowNames.index(r)
colIndex = colNames.index(c)

and raise ValueError if rowIndex or colIndex is -1. This will also work if len(rows) > 26.
If we're happy with this approach, then we don't have to make a breaking change?

[sbesson]

Very good point about rows and columns, it certainly feels more robust than what I was suggesting. In that case, the delineation would be defined as follows:

• wells as List[str] would assume the well paths can be computed from the rows and columns
• wells as List[dict] would allow to support any hierarchy and allow the sub-groups to be decoupled from the names of the rows and columns e.g. the Zarr group names could be UUIDs

Either way, I don't think there is any breaking change involved as the HCS API has not been released yet.

[will-moore]

@sbesson do you want to add support for calculating rowIndex and columnIndex like in #157 (comment) in this PR?
Just thinking about using this from omero-cli-zarr for export

[sbesson]

Yes, as per the discussion above, my intention is to support the following APIs

write_plate_metadata(plate_group, ["A", "B"], ["1", "2"], wells=["A/1", "A/2", "B/1", "B/2"])
write_plate_metadata(plate_group, ["A", "B"], ["1", "2"], wells=[{"path": "A/1", "rowIndex": 0, "columnIndex": 0}, ...)

I would add support in a follow-up PR as I'd need the FormatV04 class introduced in #124