Extended axes support by dominikl · Pull Request #28 · glencoesoftware/omero-zarr-pixel-buffer

dominikl · 2025-07-04T10:05:49Z

This PR adds support for different axes order and zarrs with less than 5 dimensions.
(Replaces #19)

Axes

Support different axes order and <5d zarrs

jburel · 2025-07-09T21:09:35Z

src/test/java/com/glencoesoftware/omero/zarr/ZarrPixelBufferTest.java

        Path output = tmpDir.getRoot().toPath().resolve("output.zarr");
-        assertBioFormats2Raw(input, output);
+        if (order != null) {
+            assertBioFormats2Raw(input, output, "--dimension-order", order);


we probably need to review that strategy

Remove usage of bioformats order argument

sbesson · 2025-08-15T15:41:07Z

As a confirmation of the ground truth, I generated a series of test files of various dimensionality using glencoesoftware/bioformats2raw#279 and ome/bioformats#4339

bioformats2raw "test&labelPlanes=true&sizeX=8192&sizeY=8192&sizeC=3.fake" XYZCT_8192x8192x1x3x1.zarr
bioformats2raw "test&labelPlanes=true&sizeX=8192&sizeY=8192&sizeC=3.fake" XYC_8192x8192x3.zarr --compact
bioformats2raw "test&labelPlanes=true.fake" XYZCT_512x512x1x1x1.zarr
bioformats2raw "test&labelPlanes=true.fake" XY_512x512.zarr --compact
bioformats2raw "test&labelPlanes=true&sizeZ=50.fake" XYZCT_512x512x50x1x1.zarr
bioformats2raw "test&labelPlanes=true&sizeZ=50.fake" XYZ_512x512x50.zarr --compact
bioformats2raw "test&labelPlanes=true&sizeC=5.fake" XYZCT_512x512x1x5x1.zarr
bioformats2raw "test&labelPlanes=true&sizeC=5.fake" XYC_512x512x5.zarr --compact
bioformats2raw "test&labelPlanes=true&sizeT=20.fake" XYZCT_512x512x1x1x20.zarr
bioformats2raw "test&labelPlanes=true&sizeT=20.fake" XYT_512x512x20.zarr --compact
bioformats2raw "test&labelPlanes=true&sizeZ=50&sizeC=5.fake" XYZCT_512x512x50x5x1.zarr
bioformats2raw "test&labelPlanes=true&sizeZ=50&sizeC=5.fake" XYZC_512x512x50x5.zarr --compact
bioformats2raw "test&labelPlanes=true&sizeZ=50&sizeT=20.fake" XYZCT_512x512x50x1x20.zarr
bioformats2raw "test&labelPlanes=true&sizeZ=50&sizeT=20.fake" XYZT_512x512x50x20.zarr --compact
bioformats2raw "test&labelPlanes=true&sizeC=5&sizeT=20.fake" XYZCT_512x512x1x5x20.zarr
bioformats2raw "test&labelPlanes=true&sizeC=5&sizeT=20.fake" XYCT_512x512x5x20.zarr --compact
bioformats2raw "test&labelPlanes=true&sizeZ=50&sizeC=5&sizeT=20.fake" XYZCT_512x512x50x5x20.zarr
bioformats2raw "test&labelPlanes=true&sizeZ=50&sizeC=5&sizeT=20.fake" XYZCT_512x512x50x5x20_2.zarr --compact

As expected, importing these Zarr datasets into OMERO (Plus) with omero-zarr-pixel-buffer 0.5.0, all the datasets with XYZCT axes are properly registered with the binary data readable while the binary data of the rest is incorrectly read

sbesson

With a build of omero-zarr-pixel-buffer including this PR deployed in OMERO.server (lib/server) as well as the image-region micro-service, all test images imported above now render as expected for all axes variants

In addition, tested the conversion and import of a couple of Zarr datasets generated from real-world examples across different modalities, already used in previous testing

384 wells plate (9 fields of view) https://downloads.openmicroscopy.org/images/Cellomics/BBBC017/
brightfield whole slide image https://downloads.openmicroscopy.org/images/Leica-SCN/openslide/
fluorescence whole slide image https://downloads.openmicroscopy.org/images/Vectra-QPTIFF/perkinelmer/
fluorescence 3D image with segmentation https://ftp.ebi.ac.uk/pub/databases/IDR/idr0079-hartmann-lateralline/20200220-ftp/experimentB/03887B26C1/

Zarr datasets were generated with and without --compact and imported into OMERO. The tile loading is consistent between the 5D and reduced dimensionality representation for all samples

Additionally, the 5D and 3D label images both render consistently

From a functional perspective, everything works as expected. The latest version of the code addresses some of the feedback raised in #19 but I'll leave @kkoz @melissalinkert who also has some knowledge of this code to run a round of code review.

From the testing perspective, since #19 (review), @melissalinkert has opened 2 contributions (mentioned previously as glencoesoftware/bioformats2raw#279 and ome/bioformats#4339) to generate sample Zarr datasets of reduced dimensionalities via bioformats2raw which can also be used for unit test. My impression is that this approach covers most of the logic of the TestZarr. Consuming it will require a Bio-Formats and bioformats2raw release. Depending on the timeline, we could integrate this as it is and capture the refactoring of the tests as a follow-up issue to consume the new functionalities?

melissalinkert · 2025-08-18T15:58:53Z

src/main/java/com/glencoesoftware/omero/zarr/ZarrPixelBuffer.java

+            }
+            if (axesOrder.containsKey(Axis.Z)) {
+                offset[axesOrder.get(Axis.Z)] = z;
+            }


Might be worth condensing lines 566-589, so that for each of the 3 axes there is something like:

if (axesOrder.containsKey(Axis.T)) { int tIndex = axesOrder.get(Axis.T); shape[tIndex] = 1; offset[tIndex] = t; }

That would reduce the number of axesOrder.containsKey and axesOrder.get calls, which at least can't hurt tile read performance.

melissalinkert · 2025-08-18T15:59:34Z

src/main/java/com/glencoesoftware/omero/zarr/ZarrPixelBuffer.java

+        }
+        if (axesOrder.containsKey(Axis.Z)) {
+            offset[axesOrder.get(Axis.Z)] = z;
+        }


As above, this could be condensed to a single containsKey(...) check for each axis.

melissalinkert · 2025-08-18T16:00:42Z

src/main/java/com/glencoesoftware/omero/zarr/ZarrPixelBuffer.java

+        }
+        if (axesOrder.containsKey(Axis.Z)) {
+            offset[axesOrder.get(Axis.Z)] = z;
+        }


Same here as for lines 566-589 and 693-715.

melissalinkert · 2025-08-18T16:05:22Z

src/test/java/com/glencoesoftware/omero/zarr/TestZarr.java

+    public TestZarr createImage() throws IOException, InvalidRangeException {
+        for (int c = 0; c <= sizeC; c++) {
+            if (c == sizeC && sizeC > 0)
+                break;


Is the idea here to execute once if sizeC is 0, and up to sizeC - 1 otherwise? If so, that might be easier to follow by removing lines 179-180, and using something like this on line 178:

for (int c = 0; c < Math.max(1, sizeC); c++)

melissalinkert · 2025-08-18T16:05:48Z

src/test/java/com/glencoesoftware/omero/zarr/TestZarr.java

+                break;
+            for (int t = 0; t <= sizeT; t++) {
+                if (t == sizeT && sizeT > 0)
+                    break;


Same question as on in the loop over sizeC.

melissalinkert · 2025-08-18T16:06:03Z

src/test/java/com/glencoesoftware/omero/zarr/TestZarr.java

+                    break;
+                for (int z = 0; z <= sizeZ; z++) {
+                    if (z == sizeZ && sizeZ > 0)
+                        break;


...and same question as for sizeC and sizeT above.

melissalinkert · 2025-08-18T16:10:18Z

src/test/java/com/glencoesoftware/omero/zarr/TestZarr.java

+        int fontSize = Math.min(sizeX, sizeY) / 20; // Scale font size based on image dimensions
+        g2d.setFont(new Font("Arial", Font.BOLD, fontSize));
+
+        String planeText = text.replace("<C>", String.valueOf(c)).replace("<T>", String.valueOf(t)).replace("<Z>", String.valueOf(z));


If text were Channel %s, Timepoint %s, Z-plane %s, then this line could be simplified to something like String planeText = String.format(text, c, t, z).

👍 Thanks @melissalinkert !

kkoz · 2025-08-19T18:55:09Z

src/main/java/com/glencoesoftware/omero/zarr/ZarrPixelBuffer.java

+            offset[axesOrder.get(Axis.Z)] = zIndexMap.get(originalZIndex);
+            planes = shape[axesOrder.get(Axis.Z)];
+            shape[axesOrder.get(Axis.Z)] = 1;


Since these require axesOrder to contain Axis.Z, should they be inside the conditional above?

kkoz · 2025-08-19T19:11:55Z

src/main/java/com/glencoesoftware/omero/zarr/ZarrPixelBuffer.java

+     * Retrieves the axes metadata of the first multiscale
+     * @return See above.
+     */
+    public Map<Axis, Integer> getAxesOrder() {


I feel like this should either be setAxesOrder and set the member variable from the zarr, or it should check to see if the value has been set and just return axesOrder if it has.

kkoz · 2025-08-19T19:27:26Z

src/main/java/com/glencoesoftware/omero/zarr/ZarrPixelBuffer.java

+            order.put(Axis.Y, 3);
+            order.put(Axis.X, 4);
+        } else {
+            for (int i=0; i<axesData.size(); i++) {


For formatting, do we want spaces between the values and operators here?

@kkoz should we review whether/how to enforce formatting on this repository as a follow up of this work?

My feeling is that if we go down this route, we should encode the decisions under the form of checkstyle constraints similarly to https://github.com/glencoesoftware/bioformats2raw/blob/97f8a26dd120d854d3e03ab70f1c481b524ff025/config/checkstyle/checkstyle.xml and let Gradle lint the code as part of the build.

I agree. I was under the impression that there was already an OME style somewhere, but after poking around a bit I couldn't find it. Automatically applying consistent formatting in all repos would definitely be a plus.

https://ome-contributing.readthedocs.io/en/latest/code-contributions.html would be the most generic guidelines and Bio-Formats has some additional instructions https://bio-formats.readthedocs.io/en/latest/developers/code-formatting.html

For OMERO, there are no written guidelines but https://github.com/ome/openmicroscopy/tree/develop/docs/styles will contain the historical style definition.

kkoz · 2025-08-19T19:42:23Z

src/main/java/com/glencoesoftware/omero/zarr/ZarrPixelBuffer.java

+                // if no Z downsampling, this is just an identity map
+                int fullResZ = fullResolutionArray.getShape()[axesOrder.get(Axis.Z)];
+                int arrayZ = array.getShape()[axesOrder.get(Axis.Z)];
+                for (int z=0; z<fullResZ; z++) {


Same formatting comment here

That's true, I've not set any particular formatting style in my IDE, sorry. Happy to use the bioformats2raw one directly!

dominikl · 2025-08-21T13:10:48Z

Addressed @kkoz suggestions. Reformatting to the bioformats2raw style rules would introduce a huge amount of changes, make review of this PR impossible. Probably better done in follow-up?

sbesson · 2025-08-21T13:20:03Z

Agreed, let's separate the formatting discussion. On this front, one of the major divergences between the Bio-Formats codebase (incl. bioformats2raw) and the OMERO codebases is the 2 spaces vs 4 spaces indentations. The value of unifying everything to one style would quickly become neglectable compared to the impact of rewriting almost every line ...

I would propose to start by reviewing the set of existing and expected rules for each component and try to identify a sensible overlap that we can enforce

melissalinkert

Minor comment aside, it looks like a couple of the review comments from @kkoz were missed as they were separate from the formatting discussion.

melissalinkert · 2025-08-21T15:11:29Z

src/test/java/com/glencoesoftware/omero/zarr/ZarrPixelBufferTest.java

+                        break;
+                    for (int c = 0; c <= sizeC; c++) {
+                        if (c == sizeC && sizeC > 0)
+                            break;


As in TestZarr lines 178-180, this would be easier to follow if using something like for (int t = 0; t < Math.max(sizeT, 1); t++).

sbesson

Retested the last commit against a set of images with XY, XYC, XYZ, XYT, XYZC, XYZT, XYZCT as well as XYZCT dimensions as well as labels with XY, XYC and ZYXCT dimensions. Images and labels are imported correctly with the thumbnails generated and the binary data is correctly read by the viewers.

@melissalinkert and @kkoz can you take a look at the code changes addressing your feedback?

dominikl and others added 24 commits May 2, 2025 13:31

Take axes metadata into account

63c9936

Add test checking default axes if not present

71c93bb

Fix order since order in omero is reverse in ome.zarr

dfe6319

Merge pull request #1 from jburel/axes

38838db

Axes

Remove printlns

d8a7e3c

Remove commented out code

7fef6bf

Remove unused imports

b6dc260

Support zarr without c, t or z axes

7d9f338

Fix swapped xy tile size

a7cd5c9

Fix issue when Z missing

0020bb6

Fix issue with checkReadSize

e4eb3e9

Add integration tests for different dimensions

ed93541

remove logging from TestZarr

8995b11

Moved classes into correct package

56df523

Fix package declaration

04fe46e

Fix order

de1096e

install blosc

51119b5

Check repo owner

0a30109

Merge pull request #2 from jburel/axes

d000619

Axes

Some refactoring

bd95004

Moved Utils method into TestZarr itself

4d9ef53

Add more missing tests

c950c9c

Use DimensionOrder

ff75695

Merge pull request #2 from dominikl/axes

a0b3c47

Support different axes order and <5d zarrs

jburel reviewed Jul 9, 2025

View reviewed changes

dominikl added 2 commits July 15, 2025 16:55

Clarify order

6945b90

Remove usage of BF --order argument

bd1bac4

dominikl mentioned this pull request Jul 15, 2025

Remove usage of bioformats order argument ome/omero-zarr-pixel-buffer#6

Merged

Merge pull request #6 from dominikl/remove_bf_dim_order

57bffda

Remove usage of bioformats order argument

will-moore mentioned this pull request Jul 24, 2025

Document that PRs should be opened again ome's fork #31

Open

sbesson mentioned this pull request Aug 14, 2025

Fake: add labelPlanes option to add human-readable labels to planes ome/bioformats#4339

Merged

sbesson linked an issue Aug 14, 2025 that may be closed by this pull request

Handle images that are not 5D #16

Closed

sbesson mentioned this pull request Aug 15, 2025

Add --compact option to reduce array dimensionality glencoesoftware/bioformats2raw#279

Merged

sbesson requested review from kkoz, melissalinkert and sbesson August 18, 2025 08:50

sbesson reviewed Aug 18, 2025

View reviewed changes

melissalinkert suggested changes Aug 18, 2025

View reviewed changes

dominikl added 2 commits August 19, 2025 16:00

Reduce calls to axesOrder.containsKey

96afc5a

Simplify code

40c09cc

kkoz reviewed Aug 19, 2025

View reviewed changes

dominikl added 2 commits August 21, 2025 13:47

Fix formatting

6194831

Sanitise getAxisOrder

3868956

sbesson requested review from kkoz and melissalinkert August 21, 2025 13:20

melissalinkert suggested changes Aug 21, 2025

View reviewed changes

dominikl added 2 commits August 22, 2025 13:12

Fix for loops

195ec0f

Move z axis access into conditional block

beeacff

sbesson approved these changes Aug 26, 2025

View reviewed changes

melissalinkert approved these changes Aug 26, 2025

View reviewed changes

sbesson mentioned this pull request Aug 26, 2025

Add clarification about the downsampling factor expectation #32

Merged

kkoz approved these changes Aug 26, 2025

View reviewed changes

sbesson merged commit 0f80674 into glencoesoftware:master Aug 26, 2025
8 of 9 checks passed

dominikl mentioned this pull request Aug 27, 2025

Support for Zarr v3 (resp NGFF 0.5) ome/omero-zarr-pixel-buffer#11

Closed

sbesson mentioned this pull request Sep 4, 2025

Coding conventions #33

Closed

sbesson mentioned this pull request Sep 16, 2025

README updates (requirements, installation & usage) #35

Merged

sbesson mentioned this pull request Sep 24, 2025

Update ZarrPixelBufferTest to use bioformats2raw 0.11.0 native features #36

Merged

Conversation

dominikl commented Jul 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbesson commented Aug 15, 2025

Uh oh!

sbesson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dominikl commented Aug 21, 2025

Uh oh!

sbesson commented Aug 21, 2025

Uh oh!

melissalinkert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sbesson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants