fractal-analytics-platform / fractal-tasks-core

Main tasks for the Fractal analytics platform
https://fractal-analytics-platform.github.io/fractal-tasks-core/
BSD 3-Clause "New" or "Revised" License
14 stars 6 forks source link

Review label/label-image use in napari-workflows-wrapper task #532

Closed tcompa closed 11 months ago

tcompa commented 11 months ago

We have this code block of the napari-workflows-wrapper task

https://github.com/fractal-analytics-platform/fractal-tasks-core/blob/e9b3996fbe78e92222d3006fe65a8e69ee99732a/fractal_tasks_core/tasks/napari_workflows_wrapper.py#L374C1-L392C14

where we are calling extract_zyx_pixel_sizes for the zattrs of a label image (rather than an image). But extract_zyx_pixel_sizes looks for the multiscales property in a zattrs, which should not be present for a label image.

How can this work? Is it actually working, or is it simply not tested?

(this comes up as part of #528, because the difference betwee image and label-image models makes it obvious that this would not work)

tcompa commented 11 months ago

Is it actually working, or is it simply not tested?

Relevant test: tests/tasks/test_workflows_napari_workflows.py::test_napari_worfklow_label_input_only

tcompa commented 11 months ago

looks for the multiscales property in a zattrs, which should not be present for a label image.

I think I got this wrong, I'll review and update the issue.

tcompa commented 11 months ago

This gets fixed right away (as in https://github.com/fractal-analytics-platform/fractal-tasks-core/commit/28d7c9ef506daa84e6ce6f28b54ed735de03ecc8) by simply stating that a image-label group is also an image group, and then can have any one of the image metadata keys (notably multiscales, but possibly also omero, axes, ...). The only difference with respect to a generic image is that it must have the image-label key.

I'm a bit confused about the reasons this doesn't happen in other libraries - see two examples below. The question then would be: can those libraries actually parse/validate one of "our" zattrs for label images? My first guess is that they do not have (yet) the appropriate Pydantic model for that.

(the ome-zarr-py case is a bit less transparent, I'll review it in the next comment)

iohub

ImageLabelMeta is defined, but I think it's not used anywhere (https://github.com/search?q=repo%3Aczbiohub-sf%2Fiohub+ImageLabelMeta&type=code), thus this is not a very relevant comparison.

pydantic-ome-ngff

They have https://github.com/JaneliaSciComp/pydantic-ome-ngff/blob/main/src/pydantic_ome_ngff/v04/imageLabel.py which correctly encodes https://ngff.openmicroscopy.org/0.4/#label-md; but then, again, it doesn't seem to appear anywhere else (https://github.com/search?q=repo%3AJaneliaSciComp%2Fpydantic-ome-ngff%20ImageLabel&type=code).

To make it clear: what I was expecting was something like

# iohub
class SomethingImageLabel(ImagesMeta):
    image_label: ImageLabelMeta

# pydantic-ome-ngff
class SomethingImageLabel(Image):
    image_label: ImageLabel
tcompa commented 11 months ago

I'm a bit confused about the reasons this doesn't happen in other libraries - see two examples below. The question then would be: can those libraries actually parse/validate one of "our" zattrs for label images? My first guess is that they do not have (yet) the appropriate Pydantic model for that.

I just realized that it's not strictly needed to enforce inheritance between LabelImage and Image. The simpler approach is to just cast the zarr attributes to a different object depending on what they represent (see f300e9a). This solution is to be preferred since it: