ome / napari-ome-zarr

A napari plugin for zarr backed OME-NGFF images
https://www.napari-hub.org/plugins/napari-ome-zarr
BSD 3-Clause "New" or "Revised" License
26 stars 21 forks source link

Labels layers don't load with napari 0.5.0 (dev) #109

Open psobolewskiPhD opened 1 month ago

psobolewskiPhD commented 1 month ago

In 0.4.19 there were a number of deprecations to labels kwargs: https://github.com/napari/napari/pull/6542 In 0.5.0 they are removed: https://github.com/napari/napari/pull/6641

This results in a traceback when trying to load ome zarr with labels, e.g. NAPARI_ASYNC=1 napari https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.4/idr0001A/2551.zarr/A/1/0 validator: https://ome.github.io/ome-ngff-validator/?source=https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.4/idr0001A/2551.zarr/A/1/0/

Traceback (most recent call last):
  File "/Users/sobolp/Documents/dev/napari/napari/components/viewer_model.py", line 1474, in _add_layer_from_data
    layer = add_method(data, **(meta or {}))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: add_labels() got an unexpected keyword argument 'color'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/sobolp/Documents/dev/napari/napari/components/viewer_model.py", line 1283, in _open_or_raise_error
    added = self._add_layers_with_plugins(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sobolp/Documents/dev/napari/napari/components/viewer_model.py", line 1401, in _add_layers_with_plugins
    added.extend(self._add_layer_from_data(*_data))
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sobolp/Documents/dev/napari/napari/components/viewer_model.py", line 1479, in _add_layer_from_data
    raise TypeError(
TypeError: _add_layer_from_data received an unexpected keyword argument ('color') for layer type labels

Env details: napari 0.5.0 from main 5c1e7281bbc11878a748add1006f7211473be16b 0.5.2 of this plugin 0.8.3 of ome-zarr python 3.11.7 macOS arm64

psobolewskiPhD commented 1 month ago

CC @jni

jni commented 1 month ago

Well, shit. I forgot and broke my rule that kwargs to our API should never need to be exotic objects. 🀦 This means that plugins have to import from napari if they want to give us this information, which imho is a big no-no.

My suggested fix is:

Additionally, we should probably (a) make a new release of napari-ome-zarr that pins napari<=0.4.19, and then when we update as above we make another release that requires napari>=0.4.19.

What do you think @psobolewskiPhD?

psobolewskiPhD commented 1 month ago

@jni I think a convenience in napari is the way to go for 0.5.0. I happened to find it here, but how many other plugins could return a labels layer? Do we want all of them to make pins?

jni commented 1 month ago

Note that plugins will break β€” it's just that the fix will be less painful.

jni commented 1 week ago

Another note: the only plugins that will break are those that specify colors for the labels β€” I expect those are in the minority. But, the fix will be pretty easy β€” colors -> colormap in the LayerDataTuple, and this will work with both 0.4.19 and 0.5.0+.

jni commented 1 week ago

See napari/napari#7025 for the proposed fix.

jni commented 1 day ago

@psobolewskiPhD probably you didn't mean for this to auto-close with my commit? πŸ˜…

psobolewskiPhD commented 1 day ago

Um, no idea how I could close this sorry.

jni commented 1 day ago

Based on the message above, it looks like my commit message had the phrase "while trying to fix [this issue]", and when you pushed to your own fork, GitHub fixated on "fix [this issue]" in your commit and was like, great, he fixed it! πŸ˜‚