Skip to content

Upgrade zarr-python dependency to 3.0 #275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Upgrade zarr-python dependency to 3.0 #275

wants to merge 16 commits into from

Conversation

aliddell
Copy link
Collaborator

@aliddell aliddell commented Mar 3, 2025

No description provided.

@aliddell aliddell linked an issue Mar 3, 2025 that may be closed by this pull request
Use zarr-python 3 #273
Open
@aliddell aliddell requested a review from ziw-liu March 3, 2025 21:07
@ziw-liu ziw-liu added NGFF OME-NGFF (OME-Zarr format) maintenance Maintenance work labels Mar 4, 2025
@ziw-liu ziw-liu added this to the 0.3.0 milestone Mar 4, 2025
@ziw-liu
Copy link
Collaborator

ziw-liu commented Mar 4, 2025

@aliddell I noticed that your editor uses 8-spaces indentation. We use black-format and 4-space indentation in this repo. Please see the contributing guide for information about setting up the styling and linting tools.

ziw-liu
ziw-liu reviewed Mar 4, 2025
View reviewed changes
iohub/ngff/nodes.py
)
)
img_arr[...] = data
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breakage seems unintended from upstream discussion. We can revert after zarr-developers/zarr-python#2819 is released.

@aliddell
Copy link
Collaborator Author

aliddell commented Mar 11, 2025 •
edited
Loading

We still have 592 591 tests failing. Of those, 589 are showing the same error

TypeError: Unsupported type for store_like: 'ZarrTiffStore'

which looks like an internal issue in tifffile. Two more (both variants of test_cli_convert_ome_tiff) are failing, also due to tifffile, e.g.,

E       AssertionError: 
E       assert 1 == 0
E        +  where 1 = <Result TypeError("Unsupported type for store_like: 'ZarrTiffStore'")>.exit_code

which just leaves one failing test, test_rename_wells, which is what was failing just before I left. This is fixed as of 61045d4.

There are likely references to the NGFF version that don't handle "0.5" or have it as an option. That needs tests.

@ziw-liu
Copy link
Collaborator

ziw-liu commented Mar 11, 2025

Thanks @aliddell! I believe the tifffile error is due to how zarr changed its interface to implement third-party storage types. We either need to wait for cgohlke/tifffile#282 or change the way we implement OME-TIFF support. However I think it's a relatively self-contained issue, and as long as the OME-Zarr part is migrated, we can continue to work on bridging acquire-zarr and iohub via Zarr v3/OME-Zarr v0.5.

@ziw-liu ziw-liu mentioned this pull request Mar 21, 2025
Merged
@ziw-liu ziw-liu mentioned this pull request Apr 3, 2025
Open
ziw-liu
ziw-liu reviewed Apr 3, 2025
View reviewed changes
iohub/_deprecated/zarrfile.py
@@ -8,6 +8,7 @@

import numpy as np
import zarr
import zarr.storage
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this import necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might not be. If I remove it, my IDE complains in line 49, but it doesn't seem to break anything.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's an implementation detail of the language server you are using.

ziw-liu added 7 commits April 4, 2025 13:49
@ziw-liu
Read MMStack with Zarr v3 (#293)
2516c9b 
* use fsspec as a wrapper for ZarrTiffStore

* update deprecated readers and tests

* document the helper function

* add back the binary check after a flag

* use kwarg for file mode in reader

* restore dtype attribute

* restore opening zarr with dask

* stringify path name

* remove redundant dtype
@ziw-liu
Merge branch 'main' into 273-use-zarr-python-3
1404000
@ziw-liu
add rich for zarr v3 print tree
2a2f75f
@ziw-liu
skip tests that require ome-zarr-py
26adb5e
@ziw-liu
fix file list checking
dd6d994 
it doesn't matter what the order is
@ziw-liu
fix commas
b9c6ccd
@ziw-liu
stringify path names
8a427c6
ziw-liu
ziw-liu reviewed Apr 4, 2025
View reviewed changes
tests/ngff/test_ngff.py
@@ -392,6 +398,7 @@ def test_rename_channel(channels_and_random_5d, arr_name, new_channel):
assert dataset.metadata.omero.channels[0].label == new_channel


# @pytest.mark.skip(reason="broken")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aliddell This test does pass now. Can you document why it was marked 'broken'?

@ziw-liu ziw-liu marked this pull request as ready for review April 4, 2025 21:26
@JoOkuma JoOkuma mentioned this pull request Apr 4, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ziw-liu ziw-liu ziw-liu left review comments

@edyoshikun edyoshikun Awaiting requested review from edyoshikun

@ieivanov ieivanov Awaiting requested review from ieivanov

@JoOkuma JoOkuma Awaiting requested review from JoOkuma

@shinzlet shinzlet Awaiting requested review from shinzlet

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
maintenance Maintenance work NGFF OME-NGFF (OME-Zarr format)
Projects
None yet
Milestone
0.3.0
Development

Successfully merging this pull request may close these issues.

Use zarr-python 3
2 participants
@aliddell @ziw-liu