Skip to content

Conversation

@AlbertDominguez
Copy link
Contributor

@AlbertDominguez AlbertDominguez commented May 23, 2025

Hi,

Thanks for the great effort you're doing here :) This PR updates the arguments passed to skimage.transform.pyramid_gaussian and skimage.transform.pyramid_laplacian (without any behaviour change) used by the Scaler class, as the arg multichannel was deprecated in scikit-image 0.19.x. Using the latest version of scikit-image (0.25.x) causes a crash if either of these methods are used for downsampling.

Note that this causes the code not to work if scikit-image<0.19. Unsure if it's worth pinning in the deps 🙂

Here's a simple code to replicate:

from ome_zarr.io import parse_url
from ome_zarr.scale import Scaler
from ome_zarr.writer import write_image
import numpy as np
import zarr

if __name__ == "__main__":
    # Example usage
    store = parse_url("dbg.ome.zarr", "w").store
    group = zarr.group(store=store, overwrite=True)
    array = np.random.randn(1,256,256)

    scaler = Scaler(
        downscale=2,
        method="gaussian", # or "laplacian"
        max_layer=2,
    )
   
    # Next line currently breaks due to deprecated args being used downstream by the Scaler instance
    write_image(
        image=array,
        group=group,
        scaler=scaler,
        axes="cyx",
    )

@joshmoore
Copy link
Member

Note that this causes the code not to work if scikit-image<0.19. Unsure if it's worth pinning in the deps 🙂

Yes, please, if you are still interested, @AlbertDominguez. With #413 finally out of the way, I imagine we can look at getting this in. Thanks!

@AlbertDominguez
Copy link
Contributor Author

Hi @joshmoore, thanks! I've just merged the latest changes into the PR + pinned scikit-image. Also further tested locally and everything seems to work as expected.

@joshmoore
Copy link
Member

Leaving this for @will-moore to look at, but I imagine we can roll it into the imminent 0.12.0.

@will-moore
Copy link
Member

Great, thanks @AlbertDominguez

I turned your test example above into a test which your PR fixes.
Could you include this in tests/test_scaler.py?

+++ b/tests/test_scaler.py
@@ -1,9 +1,12 @@
 import dask.array as da
 import numpy as np
+import pathlib
 import pytest
+import zarr
 
 from ome_zarr.scale import Scaler
-
+from ome_zarr.io import parse_url
+from ome_zarr.writer import write_image
 
 class TestScaler:
     @pytest.fixture(
@@ -146,3 +149,25 @@ class TestScaler:
         # to zarr invokes compute
         data_dir = tmpdir.mkdir("test_big_dask_pyramid")
         da.to_zarr(level_1, str(data_dir))
+
+
+    @pytest.mark.parametrize("method", ["gaussian", "laplacian"])
+    def test_pyramid_args(self, shape, tmpdir, method):
+        path = pathlib.Path(tmpdir.mkdir("data"))
+        store = parse_url(path, mode="w").store
+        group = zarr.group(store=store, overwrite=True)
+        data = self.create_data(shape)
+
+        scaler = Scaler(
+            downscale=2,
+            method=method,
+            max_layer=2,
+        )
+
+        axes = "tczyx"[-len(shape) :]
+        write_image(
+            image=data,
+            group=group,
+            scaler=scaler,
+            axes=axes,
+        )

@AlbertDominguez
Copy link
Contributor Author

Thanks @joshmoore and @will-moore! I've just added the suggested test

@codecov
Copy link

codecov bot commented Aug 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.99%. Comparing base (bc34123) to head (13bab65).
⚠️ Report is 96 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #454      +/-   ##
==========================================
- Coverage   87.02%   86.99%   -0.03%     
==========================================
  Files          13       13              
  Lines        1772     1769       -3     
==========================================
- Hits         1542     1539       -3     
  Misses        230      230              

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

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

Great, thanks

@will-moore will-moore merged commit 2eec589 into ome:master Aug 11, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants