Convert to npe2 (Fix #27 and #41)

[joshmoore]

Ran npe2 convert . per https://github.com/napari/npe2#readme

Filename_patterns are likely the biggest point of discussion.

[codecov-commenter]

Codecov Report

Merging #42 (b932d18) into main (4497294) will increase coverage by 1.31%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
+ Coverage   91.28%   92.59%   +1.31%     
==========================================
  Files           3        3              
  Lines         195      189       -6     
==========================================
- Hits          178      175       -3     
+ Misses         17       14       -3     

Impacted Files	Coverage Δ
napari_ome_zarr/_reader.py	88.77% <100.00%> (+2.23%)	:arrow_up:

---

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 4497294...b932d18. Read the comment docs.

[DragaDoncila]

@joshmoore I was actually going through updating your plugin when I found this PR 😆 . One suggestion I would have is you can now remove the napari_hook_implementation decorator and import in _reader.py. That way you can also get rid of the napari-plugin-engine dependency in setup.cfg.

[joshmoore]

Thanks, @DragaDoncila, done & pushed. (Any particular reason the converter doesn't strip those?)

[DragaDoncila]

(Any particular reason the converter doesn't strip those?)

@joshmoore I'm not 100% sure but I think it's because finding the right sections in the python file and editing them appropriately is actually way harder than editing the setup.cfg or writing a new yaml file. In fact I'm pretty sure if the plugin is using setup.py to declare the entry points, we don't edit that either because it's too tricky to parse.

Most of the introspecting work done on the functions and hook implementations is done when the plugin is installed so I don't think we actually parse the files, but rather rely on inspecting sys modules.

[joshmoore]

Makes sense. Thanks.

[joshmoore]

Propose: 0.4.0

[sbesson]

Propose: 0.4.0

NB: the tag already exists - https://github.com/ome/napari-ome-zarr/tags. I assume you got misled by the Releases section in the landing page. If so, that's likely another driver for auto-creating GitHub releases on tags?

[joshmoore]

Yes, indeed. Then 0.5.0 :)

[sbesson]

Are we changing the filename_patterns as per the discussion in https://github.com/ome/napari-ome-zarr/pull/42#discussion_r850077083 ? Or keeping it as it is given @DragaDoncila's comment that it's effectively mostly indicative since we are working with directories?

[joshmoore]

I'm of two minds. Starting with the stricter .ome.zarr seems safer since expanding later is easier. But of course the cat is at least partially out of the bag. (Depending on how the underlying mechanism does and will work)

[sbesson]

Tested in a fresh environment with

(napari) sbesson@Sebastiens-MacBook-Pro ~ % conda create -n napari -c conda-forge napari
...
(base) sbesson@Sebastiens-MacBook-Pro ~ % conda activate napari
(napari) sbesson@Sebastiens-MacBook-Pro ~ % pip install git+https://github.com/ome/napari-ome-zarr@refs/pull/42/merge
Collecting git+https://github.com/ome/napari-ome-zarr@refs/pull/42/merge
  Cloning https://github.com/ome/napari-ome-zarr (to revision refs/pull/42/merge) to /private/var/folders/jj/nbnt8dds4yjbyv02whgbl9_h0000gn/T/pip-req-build-0sq1ragn
  Running command git clone --filter=blob:none --quiet https://github.com/ome/napari-ome-zarr /private/var/folders/jj/nbnt8dds4yjbyv02whgbl9_h0000gn/T/pip-req-build-0sq1ragn
  WARNING: Did not find branch or tag 'refs/pull/42/merge', assuming revision or ref.
  Running command git fetch -q https://github.com/ome/napari-ome-zarr refs/pull/42/merge
  Running command git checkout -q eda3f342dc35925fc316c5715950f3011196e01
...
Successfully installed aiobotocore-2.2.0 aiohttp-3.8.1 aioitertools-0.10.0 aiosignal-1.2.0 async-timeout-4.0.2 botocore-1.24.21 frozenlist-1.3.0 jmespath-1.0.0 multidict-6.0.2 napari-ome-zarr-0.4.1.dev14+geda3f34 ome-zarr-0.4.1 s3fs-2022.3.0 yarl-1.7.2
(napari) sbesson@Sebastiens-MacBook-Pro ~ % napari https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.3/idr0079A/9836998.zarr
napari.manifest -> 'napari-ome-zarr' could not be imported: Could not find file 'napari.yaml' in module 'napari_ome_zarr'
Traceback (most recent call last):
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/plugins/io.py", line 118, in read_data_with_plugins
    layer_data = reader(npe1_path)  # try to read data
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/types.py", line 144, in reader_function
    result = func(*args, **kwargs)
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/utils/io.py", line 236, in magic_imread
    image, zarr_shape = read_zarr_dataset(filename)
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/utils/io.py", line 331, in read_zarr_dataset
    raise ValueError(
ValueError: Not a zarr dataset or group: https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.3/idr0079A/9836998.zarr

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

Traceback (most recent call last):
  File "/Users/sbesson/miniconda3/envs/napari/bin/napari", line 10, in <module>
    sys.exit(main())
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/__main__.py", line 447, in main
    _run()
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/__main__.py", line 312, in _run
    viewer = view_path(  # noqa: F841
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/view_layers.py", line 178, in view_path
    return _make_viewer_then('open', args, kwargs)
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/view_layers.py", line 126, in _make_viewer_then
    method(*args, **kwargs)
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/components/viewer_model.py", line 918, in open
    self._add_layers_with_plugins(
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/components/viewer_model.py", line 983, in _add_layers_with_plugins
    layer_data, hookimpl = read_data_with_plugins(
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/plugins/io.py", line 121, in read_data_with_plugins
    raise PluginCallError(result.implementation, cause=exc)
napari_plugin_engine.exceptions.PluginCallError: Error in plugin 'builtins', hook 'napari_get_reader': Not a zarr dataset or group: https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.3/idr0079A/9836998.zarr
(napari) sbesson@Sebastiens-MacBook-Pro ~ % napari --version
napari.manifest -> 'napari-ome-zarr' could not be imported: Could not find file 'napari.yaml' in module 'napari_ome_zarr'
napari version 0.4.15

Does anything need to be activated to test this PR?

[DragaDoncila]

@sbesson this is because the setup.cfg entry point lists napari_ome_zarr:napari.yaml as the path, but it's actually just at the top level. I would move napari.yaml into napari_ome_zarr as that's where we "expect" to find them, and that should resolve the issue. Alternatively you can update the setup.cfg entry point path.

[options.entry_points]
napari.manifest =
    napari-ome-zarr = napari_ome_zarr:napari.yaml #path to napari.yaml

[joshmoore]

Thanks, @DragaDoncila. Here I have to omit in the first round I used pip install . rather than pip install -e . and the napari.yaml ended up under /usr/local/anaconda3/envs/z/lib etc. I must have moved it to the wrong place when correct 🙀 Sorry for the noise.

[will-moore]

I just tried to install from this branch with pip install -e . and then I also see:

$ napari
napari.manifest -> 'napari_ome_zarr:napari.yaml' could not be imported: Could not find file 'napari.yaml' in module 'napari_ome_zarr'
/Users/wmoore/opt/anaconda3/envs/napari-env/lib/python3.9/site-packages/napari/_qt/__init__.py:53: UserWarning: 

[joshmoore]

Fixes pushed. We still need a final decision on the pattern *.zarr or *.ome.zarr.

[joshmoore]

Discussing with @sbesson, @melissalinkert, and @dgault, :+1: for having readers be lenient. We will likely try to work in terminology like this into the spec:

Readers SHOULD detect and open based on the file-ending .zarr while writers SHOULD write compatible files with the ending .ome.zarr.

[sbesson]

In the context of https://github.com/ome/ome-zarr-py/pull/195, I upgraded napari to the current version (0.4.15). With the current package releases,napari is now broken on OME-NGFF datasets

(napari) sbesson@Sebastiens-MacBook-Pro 2016-06 % napari --version
napari version 0.4.15
(napari) sbesson@Sebastiens-MacBook-Pro 2016-06 % pip freeze | grep zarr
napari-ome-zarr==0.4.0
ome-zarr==0.4.1
zarr==2.11.3
(napari) sbesson@Sebastiens-MacBook-Pro 2016-06 % napari https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.1/plates/2551.zarr
Traceback (most recent call last):
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/plugins/io.py", line 118, in read_data_with_plugins
    layer_data = reader(npe1_path)  # try to read data
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/types.py", line 144, in reader_function
    result = func(*args, **kwargs)
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/utils/io.py", line 236, in magic_imread
    image, zarr_shape = read_zarr_dataset(filename)
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/utils/io.py", line 331, in read_zarr_dataset
    raise ValueError(
ValueError: Not a zarr dataset or group: https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.1/plates/2551.zarr

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

Traceback (most recent call last):
  File "/Users/sbesson/miniconda3/envs/napari/bin/napari", line 10, in <module>
    sys.exit(main())
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/__main__.py", line 447, in main
    _run()
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/__main__.py", line 312, in _run
    viewer = view_path(  # noqa: F841
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/view_layers.py", line 178, in view_path
    return _make_viewer_then('open', args, kwargs)
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/view_layers.py", line 126, in _make_viewer_then
    method(*args, **kwargs)
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/components/viewer_model.py", line 918, in open
    self._add_layers_with_plugins(
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/components/viewer_model.py", line 983, in _add_layers_with_plugins
    layer_data, hookimpl = read_data_with_plugins(
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/plugins/io.py", line 121, in read_data_with_plugins
    raise PluginCallError(result.implementation, cause=exc)
napari_plugin_engine.exceptions.PluginCallError: Error in plugin 'builtins', hook 'napari_get_reader': Not a zarr dataset or group: https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.1/plates/2551.zarr
(napari) sbesson@Sebastiens-MacBook-Pro 2016-06 % napari --plugin napari-ome-zarr  https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.1/plates/2551.zarr
Traceback (most recent call last):
  File "/Users/sbesson/miniconda3/envs/napari/bin/napari", line 10, in <module>
    sys.exit(main())
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/__main__.py", line 447, in main
    _run()
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/__main__.py", line 312, in _run
    viewer = view_path(  # noqa: F841
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/view_layers.py", line 178, in view_path
    return _make_viewer_then('open', args, kwargs)
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/view_layers.py", line 126, in _make_viewer_then
    method(*args, **kwargs)
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/components/viewer_model.py", line 918, in open
    self._add_layers_with_plugins(
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/components/viewer_model.py", line 983, in _add_layers_with_plugins
    layer_data, hookimpl = read_data_with_plugins(
  File "/Users/sbesson/miniconda3/envs/napari/lib/python3.9/site-packages/napari/plugins/io.py", line 86, in read_data_with_plugins
    raise ValueError(
ValueError: There is no registered plugin named 'napari-ome-zarr'.
Names of plugins offering readers are: {'builtins'}

I suspect the 0.5.0 tag was simply forgotten, so I'll push one if no objection.

Possibly for a follow-up: is there a requirement on a minimal upstream napari version and is there a place to capture this?

[joshmoore]

Possibly for a follow-up: is there a requirement on a minimal upstream napari version and is there a place to capture this?

Ah, missed this follow-on. I don't think there's a spot, but :+1: in general for getting a handle on this. Not sure if @DragaDoncila has any hot tips for us 🙂

[DragaDoncila]

@joshmoore yes at minimum, for the plugin to be discovered users will need napari>=0.4.13. If you want to support older versions, you can actually add back the @napari_hook_implementation decorators and napari.plugin entry point, and then older versions of napari will load the plugin.

It's worth noting that very soon all plugins will be converted by napari to the npe2 format on discovery, so I would probably suggest recommending to users that they update their napari version to a minimum of 0.4.13, but I leave that decision up to you, of course.