napari 0.5 support

[jni]

Fixes ome/napari-ome-zarr#109

... or at least, attempts to do so!

@psobolewskiPhD, now with napari/napari#7061 merged and the code in this PR,
this doesn't crash anymore, but the labels layer never finishes loading. Do you
see the same thing? I'm not sure what the cause could be...

[jni]

I get the expected warning:

$ NAPARI_ASYNC=1 napari https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.4/idr0001A/2551.zarr/A/1/0
/Users/jni/projects/napari/napari/utils/colormaps/colormap.py:435: UserWarning: color_dict did not provide a default color. Missing keys will be transparent. To provide a default color, use the key `None`, or provide a defaultdict instance.

which confirms that the colormap dict is being correctly passed to napari... But then... Nothing. 😕

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.81%. Comparing base (9205bd7) to head (55e3328).
Report is 15 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #389      +/-   ##
==========================================
+ Coverage   85.76%   85.81%   +0.05%     
==========================================
  Files          13       13              
  Lines        1531     1537       +6     
==========================================
+ Hits         1313     1319       +6     
  Misses        218      218              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jni]

@joshmoore @will-moore I notice the message

TODO: a metadata transform should be provided by specific impls.

right above my changes, which I totally agree with 😅, indeed I was surprised that I had to come to this repo to fix this issue, but since we hope to release 0.5.0 very shortly, I was hoping to merge this fix first, do a release, and then migrate the logic to napari-ome-zarr at our leisure after that release. Any thoughts? 🙏

[joshmoore]

Happy to see a fix & release here, but I agree that this is surprising to find in this repo.

[psobolewskiPhD]

I'm a bit confused, I don't get why this fix is in this repo and not the plugin?
Here's what i observe:

1. the continued loading appears to be due to failure to broadcast -- the Labels layer has a singleton Z dim.
2. If I scroll to the 0 slice, the loading stops, but no labels are shown.

I find that this simple fix, of dropping the color key in the plugin and "translating" the metadata color key to colormap for napari, seems to work
https://github.com/ome/napari-ome-zarr/compare/main...psobolewskiPhD:napari-ome-zarr:drop_color_key?expand=1
Any thoughts?
Note: it doesn't solve the broadcast/loading issue, which might be an upstream napari issue.

[psobolewskiPhD]

BTW, i can reproduce the infinite spin without zarr/etc. so that's a napari bug -- maybe just a visual? dunno.
Made an issue: napari/napari#7070

[jni]

I find that this simple fix, of dropping the color key in the plugin and "translating" the metadata color key to colormap for napari, seems to work
ome/[email protected]:napari-ome-zarr:drop_color_key?expand=1 (compare)
Any thoughts?

(1) the changes should account for both 0.4.19x and for 0.5.0, because passing the dict only works on 0.5.0, unfortunately.
(2) I looked at both places but I found that "color" was coming from here and it is not actually an ome-zarr key from the spec, it's a translation from "colors" that I think is only on account of napari? (Based on the other things in that dict.) So I thought, (a) fix the translation and release, then (b) move that whole logic to the plugin.

Am I misunderstanding what the "color" key is doing here @will-moore @joshmoore?

[jni]

Hi @joshmoore @will-moore — napari 0.5.0 will be released in the next hour or so, at which point napari-ome-zarr will be unable to load any datasets with labels + color annotations. I guess that doesn't describe most datasets, but still, it would be good to fix this. 🙏

[will-moore]

Sorry - been away, looking now....

[jni]

nick of time 😃

Thank you! 🙏

[will-moore]

Test setup - new env etc...

$ conda create -y -n napari-env-310 -c conda-forge python=3.10
$ conda activate napari-env-310
$ python -m pip install "napari[all]<0.5.0"

# pip install -e . (from main branches of ome-zarr-py and napari-ome-zarr)

$ napari --plugin napari-ome-zarr https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.4/idr0076A/10501752.zarr

Working with labels being loaded...

Repeated with a new env using newly released napari and get:

  File "/Users/wmoore/opt/anaconda3/envs/napari5-env-310/lib/python3.10/site-packages/napari/components/viewer_model.py", line 1511, in _add_layer_from_data
    raise TypeError(
TypeError: _add_layer_from_data received an unexpected keyword argument ('color') for layer type labels

With @psobolewskiPhD's fix above in napari-ome-zarr, this is working for me again.
@psobolewskiPhD would you like to open a PR with that change? (and remove the now-invalid comment # NB: color for labels... and maybe add a comment at the other change to mention that we're mapping "color -> colormap for napari 0.5.0"

I think the easiest "fix" to make sure this doesn't break napari 0.4 users is to require napari>=0.5.0 in setup.cfg.
Is there a reason that users might want to avoid a big upgrade (for a tiny fix like this) or an easy way to avoid this? I guess we can detect the napari version but comparing version numbers etc is a bit of a pain.

[jni]

@will-moore just to clarify, you only need the changes from ome/napari-ome-zarr#112, right? This can be closed?

in this repo, can you clarify — was the color key only for napari's benefit or does it have some other function? (ie should it be removed anyway)

[joshmoore]

@will-moore just to clarify, you only need the changes from ome/napari-ome-zarr#112, right? This can be closed?

Will is away for a couple of weeks but I asked him this as well and it was a yes. I'm less clear on your other question but if you see no blockers, I say we get the other fix out.

[jni]

I do not — let's merge ome/napari-ome-zarr#112 and release!