FOV names and array names are hard-coded for the HCS dataset reader

[ziw-liu]

The v0.4 spec states:

# The name of the array is arbitrary with the ordering defined by
# by the "multiscales" metadata, but is often a sequence starting at 0.

And ome-zarr-py works with arbitrary array names in the single-FOV images layout, but not in an HCS plate layout. Only integer names starting with "0" is recognized.

It should instead be discovered from metadata (multiscales.datasets[0].path).

Also, FOV names not starting with "0" is not supported. The cause is likely this line hard-coding 'first field':

ome-zarr-py/ome_zarr/reader.py

Line 493 in c1302e0

self.first_field = "0"

It should instead be discovered from metadata (well.images[0].path). Spec:

The well dictionary MUST contain an images key whose value MUST be a list of JSON objects specifying all fields of views for a given well. Each image object MUST contain a path key whose value MUST be a string specifying the path to the field of view. The path MUST contain only alphanumeric characters, MUST be case-sensitive, and MUST NOT be a duplicate of any other path in the images list. 

Edit: separate description for array and position names.

[ziw-liu]

According to #200 (comment), one can view all the FOVs by loading individual wells. However this only works for incremental integer FOV path names, not any 'case sensitive, alphanumeric' path names defined in the specification. Possible cause:

ome-zarr-py/ome_zarr/reader.py

Lines 427 to 431 in c1302e0

	def get_field(tile_name: str, level: int) -> np.ndarray:
	"""tile_name is 'row,col'"""
	row, col = (int(n) for n in tile_name.split(","))
	field_index = (column_count * row) + col
	path = f"{field_index}/{level}"

[will-moore]

Thanks for the feedback.
You're right that there is an assumption made about the path to first image in a Well.
It would be a pretty easy fix to load the Well images for a Well and read that data. It's a bit of a bigger change to read that data for ALL Wells in a plate if you can't assume that all Wells have the same path to first image.

This is a similar type of change to #241 where it becomes slower to load plate data if you don't make assumptions about all the Wells being the same.

Reading Well images may not have such a big impact as the changes on that PR, but it raises similar questions:
For plates where all Wells have the same path to first image, we would prefer NOT to have to load the Images for every Well to check.

@ziw-liu In your data, does each Well have a different path to the first image?
If so, then I guess we first need to test and see how much slower it is to read that for each image.

[ziw-liu]

@will-moore Thanks for the explanation.

I agree that hardcoding is the fastest way to get the images. As for the dataset I have in mind, all the wells would have the same path (in fact '0') to the first image. I was trying to load synthetic data (generated by testing a custom writer) with the viewer, and found that writing with different FOV names that should be valid according to the specification is only supported by the ome-zarr-py reader when loading a single well.

The array name finding is of a bit more real-world concern. In our workflow it is common to transform the raw data into other arrays with incompatible data types and shapes, and it would be a cleaner data management practice if these can all be held in the same Zarr group with their names indicating their nature (e.g. 'raw', 'projection'). It is possible to rename the arrays we want to load with ome-zarr-py and napari-ome-zarr to '0' so that they are recognized, but it would be nice if the NGFF specification is fully implemented here, and data can be accessed with Zarr-style name keys when not using ome-zarr-py.

As for the performance concern, I don't have an intuition for cloud object storage, but for a storage server in the local netowrk, increased I/O operations should only be linear to the image count to load the .zattrs files. But of course only testing can give a real answer.

[will-moore]

Thanks for the info. This makes good sense.
Do you have a 'multiscales' object for e.g. raw and projection, each with a pyramid of 'datasets'?
Currently, only the first multiscales is object is read by ome-zarr-py.

[ziw-liu]

Do you have a 'multiscales' object for e.g. raw and projection, each with a pyramid of 'datasets'?

I do not think we will have multiple pyramids for a single FOV, and I agree that for visualization only loading the first pyramid makes sense. Are you suggesting that incompatible arrays can be stored in separate multiscales objects?

[will-moore]

The spec allows multiple multiscales images https://ngff.openmicroscopy.org/latest/#multiscale-md and doesn't say anything about the relationship between them.
However, currently all NGFF tools assume only 1 multiscales image per NGFF file and ignore any others: (see 'multiple multiscales' at https://ome.github.io/ome-ngff-tools/)

[aeisenbarth]

I fell into the same problem. Well image path and field index seem to be used a bit interchangeably in the code, but this is not reflected in any document, especially the spec. For my use case, I'd prefer to have human-readable image names, because these are displayed, e.g. in Napari.

If the reason are performance concerns about accessing image paths when reading the plate, then maybe the paths should not (only) live at the well level, or the spec could be modified not to allow non-index image names.

Since ome-zarr-py already has assumptions (not fully implemented spec), it's a matter of weighing up the probability that a dataset has well image path != field index versus the probability that the first well's image paths are not the same for all wells. I think the second is less likely and a fix would increase the amount of supported datasets.

[ziw-liu]

As of v0.10.3 I'm still seeing this bug. Consider this test case which passes:

def test_fov_name_extern(tmp_path):
    import numpy as np
    import zarr

    from ome_zarr.io import parse_url
    from ome_zarr.writer import (
        write_image,
        write_plate_metadata,
        write_well_metadata,
    )
    from ome_zarr.reader import Reader

    path = tmp_path / "test_ngff_plate.zarr"
    images = (
        ("A", "1", "0"),
        ("B", "1", "0"),
        ("H", "12", "0"),
    )
    row_names = ["A", "B", "H"]
    col_names = ["1", "12"]
    well_paths = [f"{row}/{col}" for row, col, _ in images]
    shape = (4, 128, 128)

    store = parse_url(path, mode="w").store
    root = zarr.group(store=store)
    write_plate_metadata(root, row_names, col_names, well_paths)
    for wi, wp in enumerate(well_paths):
        row, col = wp.split("/")
        row_group = root.require_group(row)
        well_group = row_group.require_group(col)
        field = images[wi][2]
        write_well_metadata(well_group, field)
        image_group = well_group.require_group(field)
        write_image(
            image=np.ones(shape),
            group=image_group,
            axes="zyx",
            storage_options=dict(chunks=shape),
        )
    list(Reader(parse_url(path))())

With a simple change:

    images = (
        ("A", "1", "0"),
        ("B", "1", "0"),
-       ("H", "12", "0"),
+       ("H", "12", "1"),
    )

The test fails because ome-zarr-py is still trying to look for "H/12/0":

self = <zarr.storage.FSStore object at 0x10c676c90>, key = 'H/12/0/.zgroup'
value = b'{\n    "zarr_format": 2\n}'

    def __setitem__(self, key, value):
        if self.mode == "r":
>           raise ReadOnlyError()
E           zarr.errors.ReadOnlyError: object is read-only

.../site-packages/zarr/storage.py:1463: ReadOnlyError

I think this issue needs to be re-opened, since this behavior is not only a spec conformance bug but also a round trip failure.

[will-moore]

Agreed that the current behaviour doesn't follow the spec exactly, because of the trade-off with performance.

We could load the Well fields for every Well, but that could be a lot of extra calls which would be expected to slow down the loading of a plate. How much slower I don't know, but we probably don't want to do it for ALL plates, since many won't need it.

One option that occurs to me is that for every Well, we could try to get the image data using the first_field_path (as we do now), but if that fails, then instead of returning zeros, we could load the Well fields and pick the first at

https://github.com/aeisenbarth/ome-zarr-py/blob/9d247f4eaaa2e97e3bb77cc735add8285627c64f/ome_zarr/reader.py#L549

This would have no impact on performance when every Well uses the same first_field_path, but would handle the odd Well where the field path(s) are different.

For a plate where most Wells use a different field path from the first Well, this try/except strategy could be slower than simply looking up the field paths for every Well first. If we know that the try/except is going to fail for most Wells then it's best not to waste time with it. But in the case where we only need to handle exceptional Wells, then this feels like a nice solution.

Thoughts @ziw-liu @jluethi ?