Skip to content

Ready for 06 - #605

Open
jo-mueller wants to merge 17 commits into
ome:masterfrom
jo-mueller:ready-for-06
Open

Ready for 06#605
jo-mueller wants to merge 17 commits into
ome:masterfrom
jo-mueller:ready-for-06

Conversation

@jo-mueller

@jo-mueller jo-mueller commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

@will-moore maybe one more thing before shipping the latest release. This PR

  • replaces 0.6.dev4 by 0.6 in the written and read versions
  • adds some more checks. Prior to this PR, some of the version check clauses missed the check for 0.6 so some of the additional metadata (i.e., omero and image-label) would never be ingested in case a version 0.6x were encountered.
  • Adds 0.6 in the tests more broadly

Rebased on #609

@jo-mueller jo-mueller added the bug Something isn't working label Jul 24, 2026
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.51%. Comparing base (e65666e) to head (89ae7f8).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #605      +/-   ##
==========================================
+ Coverage   86.46%   86.51%   +0.04%     
==========================================
  Files          16       16              
  Lines        2365     2366       +1     
==========================================
+ Hits         2045     2047       +2     
+ Misses        320      319       -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread tests/test_writer.py
# Validate with ome-zarr-models-py: only supports v0.4
Models04Image.from_zarr(out)
elif version.version == "0.5":
Models05Image.from_zarr(out)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to remove these checks?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We don't have to, but since the metadata is essentially ingested with ome-zarr-models-py, we are kind of already validating on-the-fly

Comment thread ome_zarr/classes/image.py Outdated
@will-moore

Copy link
Copy Markdown
Member

In order that the write_image(data) method creates v0.6 data, we need a FormatV06 class to be the CurrentFormat.
If we also add this to the tests, that looks like...

diff --git a/ome_zarr/format.py b/ome_zarr/format.py
index 3b2c33e..583fe18 100644
--- a/ome_zarr/format.py
+++ b/ome_zarr/format.py
@@ -384,4 +384,14 @@ class FormatV05(FormatV04):
         return {"name": "default", "separator": "/"}
 
 
-CurrentFormat = FormatV05
+class FormatV06(FormatV05):
+    """
+    Changelog: added FormatV06 (May 2025): writing not supported yet
+    """
+
+    @property
+    def version(self) -> str:
+        return "0.6"
+
+
+CurrentFormat = FormatV06
diff --git a/tests/test_writer.py b/tests/test_writer.py
index 836b746..4c7bb58 100644
--- a/tests/test_writer.py
+++ b/tests/test_writer.py
@@ -31,6 +31,7 @@ from ome_zarr.format import (
     FormatV03,
     FormatV04,
     FormatV05,
+    FormatV06,
     format_from_version,
 )
 from ome_zarr.scale import _build_pyramid
@@ -58,6 +59,7 @@ TRANSFORMATIONS = [
 FORMAT_VERSIONS = [
     pytest.param(FormatV04, id="V04"),
     pytest.param(FormatV05, id="V05"),
+    pytest.param(FormatV06, id="V06"),
 ]

This creates a few failures which I haven't looked into yet....

@jo-mueller

Copy link
Copy Markdown
Collaborator Author

In order that the write_image(data) method creates v0.6 data, we need a FormatV06 class to be the CurrentFormat.

right...I was hoping to remove the Format class at some point (all of its functionality is now present elsewhere). But for the sake of going forward with 0.6, I could add the FormatV06 class here and mark it for later removal?

@jo-mueller

Copy link
Copy Markdown
Collaborator Author

This creates a few failures which I haven't looked into yet....

oh boy. Jep, quite a few. I think this requires a bit of refactoring until all functions that are writing data somehow are 0.6 compliant one way or another 🙈

@jo-mueller

jo-mueller commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

@will-moore ok, this is turning out super-hard, because there are still more than writer paths (class-based, write_image/write_labels, write_multiscale/write_multiscale_labels), all of which would somehow need to be updated to write 0.6, where only the first one (class-based) is really fit for it at this point. And I kind of don't want to update code that we want to deprecate in the long run anyway.

So my suggestion:

  • I add an option in the OMEZarrMultiscale class that it can be constructed either
    • from a single OMEZarrImage and the pyramid is built under the hood. The write_image and write_labels already use that under the hood.
    • from a list of OMEZarrImages: This would be new and would be equivalent to writing a pre-computed pyramid. This could then be used in the write_multiscale and write_multiscale_labels function

This way, we would get a few props

  • The testing data for all these three paths (class based, write_image, write_multiscales) would effectively take the same path through the writer and could be simplified
  • Compliance with 0.6 would need to sit only at one place (inside the OMEZarrMultiscale class)
  • There are cases where creating OMEZarrMultiscale from pre-computed pyramids is very desirable, for instance for (down-)loading data. In this case, the resolution levels are effectively precomputed and one would decidedly not want to create the downscaled copies from a potentially large image. If we were to do this, we would download a big image and downscale instead of downloading a small one to begin with.

In the meantime, I would revert the changes here to dc07c55 (before the introduction of the FormatV06 class) so we can ship the pre-release.

@jo-mueller

Copy link
Copy Markdown
Collaborator Author

Oh no. And tests are failing with something compression related 💀

@will-moore

Copy link
Copy Markdown
Member

Just trying to see what changed in the failing tests, looking at installed versions. Since the tests passed previously, zarr 3.3.0 has been released, and it's only installed on python 3.12 and 3.13 (failing tests). Older zarr is used on the py 3.11 tests which are passing. But I don't see anything obvious in https://github.com/zarr-developers/zarr-python/releases/tag/v3.3.0

@will-moore

Copy link
Copy Markdown
Member

Your suggestion sounds good.

Re: deprecating Format classes - I see that image.to_ome_zarr() creates FormatV05() when writing v0.6 because the underlying _write_pyramid_to_zarr() method expects a Format, so trying to untangle these is a going to be a bit of work.

@jo-mueller

Copy link
Copy Markdown
Collaborator Author

so trying to untangle these is a going to be a bit of work.

Jep. My approach is to remove the metadata writing entirely from the _write_pyramid_to_zarr entirely, because the written metadata there is overwritten by the ozmp model_dump later anyway. That will make the write_multiscale_metadata obsolete.

_write_pyramid_to_zarr will only need to check the format to see whether it needs to go for zarr v2 or zarr v3. PR coming soon!

@jo-mueller

Copy link
Copy Markdown
Collaborator Author

For now, I'll pin zarr-python<3.3.0

Comment thread pyproject.toml Outdated
@jo-mueller

Copy link
Copy Markdown
Collaborator Author

Ok, this is looking good. Pinging @d-v-b here - maybe he knows whether something has changed on the zarr side that could have breaking effects? For reference, tests here started failing in a9da128 after the release of zarr-python 3.3.0.

@d-v-b

d-v-b commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

can you link me to a log with the test failures in it?

@jo-mueller

Copy link
Copy Markdown
Collaborator Author

@d-v-b

d-v-b commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

got it, fix incoming

d-v-b and others added 2 commits July 31, 2026 16:57
zarr 3.3.0 changed codec configuration attributes such as
`BloscCodec.cname` and `BytesCodec.endian` from `Enum` members to plain
strings, breaking the `.name` accesses in the label writer storage
options tests (12 failures).

Add a `_codec_value` helper that normalizes both representations, so the
tests pass against zarr 3.2 and 3.3 alike. Test-only change; the library
code never accessed these attributes.

Assisted-by: ClaudeCode:claude-opus-5
@jo-mueller

Copy link
Copy Markdown
Collaborator Author

Ok, will rebase on #609 or wait until that is in.

@jo-mueller jo-mueller mentioned this pull request Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants