Update reader.py to v0.3 #89

constantinpape · 2021-06-11T21:28:03Z

Changes for upcoming multiscale spec v0.3

…interpolation

for more information, see https://pre-commit.ci

constantinpape · 2021-06-14T12:04:44Z

I have also removed the cv2 dependency here now, see discussion in #90.
However, I think it makes sense to wait with this PR on #80 and include these changes (and also wait on #91).
@joshmoore do you have an ETA on #80?

Edit: We should also add a test for the nearest neighbor downscaling here.

joshmoore · 2021-06-14T12:07:24Z

re: 36e37cf cc: @will-moore and @erindiel

joshmoore · 2021-06-14T12:08:34Z

do you have an ETA on #80?

@constantinpape : I'll re-check it now, but I assume it can go in. It may not be 100%, but having a third class to compare to may actually help get the API right.

constantinpape · 2021-06-14T12:55:44Z

@constantinpape : I'll re-check it now, but I assume it can go in. It may not be 100%, but having a third class to compare to may actually help get the API right.

Ok. I think it's best if you just merge #80 when you think it should work.
I will then adapt this PR to the changes and we can discuss any potential issues and potentially do changes here.

… into patch-1

for more information, see https://pre-commit.ci

ome_zarr/reader.py

… into patch-1

codecov · 2021-07-07T20:07:30Z

Codecov Report

Merging #89 (93d0964) into master (a67ed4f) will increase coverage by 0.93%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master      #89      +/-   ##
==========================================
+ Coverage   69.37%   70.30%   +0.93%     
==========================================
  Files          11       11              
  Lines        1035     1044       +9     
==========================================
+ Hits          718      734      +16     
+ Misses        317      310       -7

Impacted Files	Coverage Δ
ome_zarr/reader.py	`58.43% <85.71%> (+0.27%)`	⬆️
ome_zarr/format.py	`94.52% <100.00%> (+0.40%)`	⬆️
ome_zarr/io.py	`83.03% <100.00%> (+0.89%)`	⬆️
ome_zarr/scale.py	`60.57% <100.00%> (+6.29%)`	⬆️

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 a67ed4f...93d0964. Read the comment docs.

constantinpape · 2021-07-07T20:15:35Z

@joshmoore I finally have some time again to look into this.
The following things are left to do for this PR:

Fix the precommit; I had a quick look, but I don't know what's going wrong there.
Add tests for older format versions (multiscales 0.1, 0.2), add roundtrip tests that make sure that the data written is read back correctly
Fix issues for 0.1 (0.2?); I can see that it does not work properly when checking visually with Enable opening spec v3 napari-ome-zarr#8. Might be related to issues with nested vs. flat directory store.
Update write_multiscales and Scaler to work for <5d.
Investigate issues with ome-zarr info / ome-zarr download. The tests seem to pass, BUT when I try it with this PR I get 404s for https://s3.embassy.ebi.ac.uk/idr/zarr/v0.1/6001240.zarr/, whereas this works on main.

joshmoore · 2021-07-14T09:21:50Z

ome_zarr/scale.py

-            dsize=(sizeY // self.downscale, sizeX // self.downscale),
-            interpolation=cv2.INTER_NEAREST,
-        )
+            output_shape=(sizeY // self.downscale, sizeX // self.downscale),


@will-moore : can you check that this matches your cv2 use cases?

I think the output_shape needs to be (sizeX, sizeY) as in https://github.com/ome/ome-zarr-py/pull/71/files#diff-47d7162e6c0191b1b2d4a31da6e1445085655c7a1e63354b488b1d53ec54d1a2R421
See examples at https://www.tutorialkart.com/opencv/python/opencv-python-resize-image/ with dim = (width, height)

Ok, I will double check this and see if we can cover it by some test.

I have added tests for the scaler in e07194d. Judging from the results the current implementation is correct.

ome_zarr/format.py

will-moore · 2021-07-14T10:46:15Z

@constantinpape with ome/napari-ome-zarr#9 I can use this PR to open v0.3 images in napari (tested with 1 image), but this expects the axes in the metadata, so it needs a small addition to your PR:

e.g. this works:

diff --git a/ome_zarr/reader.py b/ome_zarr/reader.py
index 0d4019d..d9f5043 100644
--- a/ome_zarr/reader.py
+++ b/ome_zarr/reader.py
@@ -273,6 +273,7 @@ class Multiscales(Spec):
             axes = tuple(multiscales[0].get("axes", ["t", "c", "z", "y", "x"]))
             if len(set(axes) - axes_values) > 0:
                 raise RuntimeError(f"Invalid axes names: {set(axes) - axes_values}")
+            node.metadata['axes'] = axes
             datasets = [d["path"] for d in datasets]
             self.datasets: List[str] = datasets
             LOGGER.info("datasets %s", datasets)

ome_zarr/reader.py

joshmoore · 2021-08-06T10:02:07Z

Hi @constantinpape, do you have a feeling when you want to try to get back to this PR? We have a hand-full that are all interlinked (below) and can start getting pre- versions ready, but just want to time things right.

cc: @manzt

constantinpape · 2021-08-06T10:03:44Z

Hi @joshmoore,
yes I will try to get back to this next week.
The last weeks were more busy with modelzoo related things than expected so I didn't have time for this; but I should be free to work on ome.zarr related things on Monday.

Co-authored-by: Will Moore <[email protected]>

constantinpape · 2021-08-11T10:56:16Z

I found another issue with the CLI: files in format v0.3 are downloaded with flat instead of nested chunks:

$ ome_zarr download https://minio-dev.openmicroscopy.org/idr/v0.3/idr0077-valuchova-flowerlightsheet/9836842.zarr
downloading...
   9836842.zarr
to .
[########################################] | 100% Completed |  0.3s
[########################################] | 100% Completed |  0.2s
[########################################] | 100% Completed |  0.2s
[########################################] | 100% Completed |  0.6s
[########################################] | 100% Completed |  0.9s
[########################################] | 100% Completed |  1.8s
$ ls 9836842.zarr/0/
0.0.0  1.0.0  2.0.0  3.0.0

constantinpape · 2021-08-11T13:11:56Z

@will-moore @joshmoore I found the issue with ome_zarr info; it was just a typo.

The problem with ome_zarr download seems to come when the zarr arrays are serialized with data.to_zarr here. So this looks like a dask or zarr-python issue, which might be solved when the issues with the zarr dimension-separator are fixed.

I think this PR is good to be merged, pending minor TODOs:

Mark failing tests as expected failures
Can I ignore the pre-commit failures? If not I would need some help in fixing this as I don't understand what's going on there.

Let me know what you think.

joshmoore · 2021-08-11T13:22:11Z

👍

So this looks like a dask or zarr-python issue, which might be solved when the issues with the zarr dimension-separator are fixed.

Possibly, but a heads up that I ran into something similar with trying to use zarr.convenience methods for converting from flat to nested (for example). Some internal methods don't go through the store themselves and instead copy internals directly leading to possible issues.

Can I ignore the pre-commit failures?

Sure. I can fix those.

Mark failing tests as expected failures

👍 If you can get a state that's green as you expect things to work, then we can deal with everything else downstream.

for more information, see https://pre-commit.ci

constantinpape · 2021-08-11T14:25:47Z

+1 If you can get a state that's green as you expect things to work, then we can deal with everything else downstream.

Ok, it's almost there; the only thing that's sometimes failing is the test for ome_zarr info, I assume due to too many requests with all the separate test runs. I would suggest to ignore this, as we can decrease the number of test runs significantly once vispy (and in consequence PyQT / PySide) is removed from the dependencies.

… into patch-1

constantinpape · 2021-08-11T15:13:11Z

All green except the pre-commit :).

tests/test_cli.py

tests/test_writer.py

constantinpape · 2021-08-12T11:53:04Z

This looks good now! @joshmoore do you want to draft releases here and in napari-ome-zarr before we merge this one?

constantinpape · 2021-08-13T15:36:14Z

Just fyi, I am on vacations for the next two weeks, feel free to merge whenever this fits or change things if necessary. And looking forward to working v0.3 python libraries when I am back ;).

With the 2.9 series of zarr-python, both dimension separators should now be functional. This will let us remove the spec restriction in future versions.

joshmoore · 2021-08-25T08:02:53Z

Pushed a commit to test zarr 2.9.x. Otherwise, I think this is ready to go but just in case there are any last thoughts, now is the time.

joshmoore · 2021-08-25T13:37:35Z

Moving forward with 0.0.22.

Update reader.py to v0.3

10df6d4

constantinpape marked this pull request as draft June 11, 2021 21:28

constantinpape mentioned this pull request Jun 14, 2021

Bloated dependencies and installation #90

Closed

constantinpape and others added 3 commits June 14, 2021 13:29

Merge remote-tracking branch 'ome/master' into patch-1

0e5dc42

Remove opencv dependency and use skimage.resize for nearest-neighbor …

36e37cf

…interpolation

[pre-commit.ci] auto fixes from pre-commit.com hooks

cfe03e3

for more information, see https://pre-commit.ci

joshmoore mentioned this pull request Jun 14, 2021

Upgrade v0.1 to v0.2 #80

Merged

3 tasks

constantinpape and others added 4 commits June 15, 2021 09:28

Merge remote-tracking branch 'ome/master' into patch-1

eac41be

Add FormatV03

f28b051

Merge branch 'patch-1' of https://github.com/constantinpape/ome-zarr-py…

32aaa47

… into patch-1

[pre-commit.ci] auto fixes from pre-commit.com hooks

49017d7

for more information, see https://pre-commit.ci

joshmoore reviewed Jun 16, 2021

View reviewed changes

ome_zarr/reader.py Show resolved Hide resolved

constantinpape added 3 commits July 7, 2021 11:08

Merge branch 'master' into patch-1

1129149

Fix tests

5740ba1

Merge branch 'patch-1' of https://github.com/constantinpape/ome-zarr-py…

41a6c6a

… into patch-1

constantinpape mentioned this pull request Jul 7, 2021

Enable opening spec v3 ome/napari-ome-zarr#8

Closed

joshmoore reviewed Jul 14, 2021

View reviewed changes

ome_zarr/format.py Show resolved Hide resolved

will-moore mentioned this pull request Jul 14, 2021

Use 'axes' metadata to pick channel_axis ome/napari-ome-zarr#9

Merged

will-moore mentioned this pull request Jul 14, 2021

export v0.3 ome ngff ome/omero-cli-zarr#70

Merged

will-moore reviewed Jul 14, 2021

View reviewed changes

ome_zarr/reader.py Show resolved Hide resolved

Update ome_zarr/reader.py

0ba85e7

Co-authored-by: Will Moore <[email protected]>

Fix typo in ZarrLocation

329c852

constantinpape requested review from joshmoore and will-moore August 11, 2021 13:12

constantinpape and others added 2 commits August 11, 2021 16:06

Mark expected failures with xfail

5dfd973

[pre-commit.ci] auto fixes from pre-commit.com hooks

d9f994d

for more information, see https://pre-commit.ci

constantinpape mentioned this pull request Aug 11, 2021

Wrap colormap in vispy object ome/napari-ome-zarr#10

Merged

constantinpape added 2 commits August 11, 2021 16:37

Merge remote-tracking branch 'ome/master' into patch-1

0fc3318

Merge branch 'patch-1' of https://github.com/constantinpape/ome-zarr-py…

a60d040

… into patch-1

Cherry-pick isory fix: 561df25

84d8a54

joshmoore reviewed Aug 12, 2021

View reviewed changes

tests/test_cli.py Outdated Show resolved Hide resolved

Add v0.2 test data

79e53c0

joshmoore reviewed Aug 12, 2021

View reviewed changes

tests/test_writer.py Show resolved Hide resolved

Remove xfail after dim separator fix (Close ome#104)

cf1d243

With the 2.9 series of zarr-python, both dimension separators should now be functional. This will let us remove the spec restriction in future versions.

joshmoore added 2 commits August 25, 2021 10:04

Skip fsspec 2021.7.0

741e952

Use official upload of v0.3 data

93d0964

joshmoore force-pushed the patch-1 branch from 67311a0 to 93d0964 Compare August 25, 2021 13:31

joshmoore merged commit 62b49da into ome:master Aug 25, 2021

sbesson mentioned this pull request Sep 1, 2021

Remove opencv dependency ome/omero-cli-zarr#80

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update reader.py to v0.3 #89

Update reader.py to v0.3 #89

constantinpape commented Jun 11, 2021

constantinpape commented Jun 14, 2021 •

edited

Loading

joshmoore commented Jun 14, 2021

joshmoore commented Jun 14, 2021

constantinpape commented Jun 14, 2021

codecov bot commented Jul 7, 2021 •

edited

Loading

constantinpape commented Jul 7, 2021 •

edited

Loading

joshmoore Jul 14, 2021

will-moore Jul 14, 2021

constantinpape Aug 10, 2021

constantinpape Aug 10, 2021

will-moore commented Jul 14, 2021

joshmoore commented Aug 6, 2021 •

edited

Loading

constantinpape commented Aug 6, 2021

constantinpape commented Aug 11, 2021

constantinpape commented Aug 11, 2021

joshmoore commented Aug 11, 2021

constantinpape commented Aug 11, 2021

constantinpape commented Aug 11, 2021

constantinpape commented Aug 12, 2021

constantinpape commented Aug 13, 2021

joshmoore commented Aug 25, 2021

joshmoore commented Aug 25, 2021

Update reader.py to v0.3 #89

Update reader.py to v0.3 #89

Conversation

constantinpape commented Jun 11, 2021

constantinpape commented Jun 14, 2021 • edited Loading

joshmoore commented Jun 14, 2021

joshmoore commented Jun 14, 2021

constantinpape commented Jun 14, 2021

codecov bot commented Jul 7, 2021 • edited Loading

Codecov Report

constantinpape commented Jul 7, 2021 • edited Loading

joshmoore Jul 14, 2021

Choose a reason for hiding this comment

will-moore Jul 14, 2021

Choose a reason for hiding this comment

constantinpape Aug 10, 2021

Choose a reason for hiding this comment

constantinpape Aug 10, 2021

Choose a reason for hiding this comment

will-moore commented Jul 14, 2021

joshmoore commented Aug 6, 2021 • edited Loading

constantinpape commented Aug 6, 2021

constantinpape commented Aug 11, 2021

constantinpape commented Aug 11, 2021

joshmoore commented Aug 11, 2021

constantinpape commented Aug 11, 2021

constantinpape commented Aug 11, 2021

constantinpape commented Aug 12, 2021

constantinpape commented Aug 13, 2021

joshmoore commented Aug 25, 2021

joshmoore commented Aug 25, 2021

constantinpape commented Jun 14, 2021 •

edited

Loading

codecov bot commented Jul 7, 2021 •

edited

Loading

constantinpape commented Jul 7, 2021 •

edited

Loading

joshmoore commented Aug 6, 2021 •

edited

Loading