Skip to content

Fix: ZarrAvgMerger ValueError with zarr_format 3 #8477

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.

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

Merged
merged 26 commits into from
Jun 13, 2025

Conversation

kolasaniv1996
Copy link
Contributor

@kolasaniv1996 kolasaniv1996 commented Jun 9, 2025

This PR fixes the issue with ZarrAvgMerger when using zarr v3, where the compressor argument is not supported.

Changes:

  • Added deprecated_arg decorator for compressor, value_compressor, and count_compressor (since 1.5.0, to be removed in 1.7.0)
  • Added codecs, value_codecs, and count_codecs support for zarr format 3
  • Updated tests to handle both zarr v2 and zarr v3 compatibility
  • Fixed issue where tests would fail with zarr v3 due to compressor usage

This addresses the issue reported in #8476 where using compressor with zarr v3 causes ValueError.

Signed-off-by: kolasaniv1996 [email protected]

@kolasaniv1996
Copy link
Contributor Author

@KumoLiu @ericspod @Nic-Ma Please review the changes when you get a chance

@ericspod
Copy link
Member

My understanding was that this argument was left in place to be compatible with zarr 2 as well as zarr 3. There was discussion on this here. A related issue raised in #8476 in our tests occurs when using zarr 3 with our tests, it would be nice to integrate a fix in this test by adding logic to remove this argument here and replace it with a use of the codecs argument (I think).

@KumoLiu
Copy link
Contributor

KumoLiu commented Jun 12, 2025

Hi @kolasaniv1996, I wanted to check in to see if you’re planning to complete this PR. It seems it might be a blocker for our v1.5 release. Let me know if there's anything we can do to assist. Thanks.

@kolasaniv1996
Copy link
Contributor Author

@KumoLiu I'm working on it

kolasaniv1996 and others added 3 commits June 12, 2025 02:06
- Added deprecated_arg decorator for compressor, value_compressor, and count_compressor (since 1.5.0, to be removed in 1.7.0)

- Added codecs, value_codecs, and count_codecs support for zarr format 3

- Updated tests to handle both zarr v2 and zarr v3 compatibility

- Fixed issue where tests would fail with zarr v3 due to compressor usage

Signed-off-by: kolasaniv1996 <[email protected]>
@KumoLiu
Copy link
Contributor

KumoLiu commented Jun 12, 2025

CI error log:

======================================================================
ERROR: test_deprecated_compressor_warning (tests.inferers.test_zarr_avg_merger.ZarrAvgMergerTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/storage.py", line 562, in _init_array_metadata
    compressor_config = compressor.get_config()
AttributeError: 'str' object has no attribute 'get_config'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/runner/work/MONAI/MONAI/tests/inferers/test_zarr_avg_merger.py", line 412, in test_deprecated_compressor_warning
    ZarrAvgMerger(merged_shape=TENSOR_4x4.shape, compressor="LZ4")
  File "/home/runner/work/MONAI/MONAI/monai/utils/module.py", line 477, in _wrapper
    return call_obj(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/utils/deprecate_utils.py", line 223, in _wrapper
    return func(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/utils/deprecate_utils.py", line 223, in _wrapper
    return func(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/utils/deprecate_utils.py", line 223, in _wrapper
    return func(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/inferers/merger.py", line 349, in __init__
    self.output = zarr.empty(
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/creation.py", line 317, in empty
    return create(shape=shape, fill_value=None, **kwargs)
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/creation.py", line 209, in create
    init_array(
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/storage.py", line 455, in init_array
    _init_array_metadata(
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/storage.py", line 564, in _init_array_metadata
    raise BadCompressorError(compressor) from e
zarr.errors.BadCompressorError: bad compressor; expected Codec object, found 'LZ4'

======================================================================
ERROR: test_deprecated_count_compressor_warning (tests.inferers.test_zarr_avg_merger.ZarrAvgMergerTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/storage.py", line 562, in _init_array_metadata
    compressor_config = compressor.get_config()
AttributeError: 'str' object has no attribute 'get_config'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/runner/work/MONAI/MONAI/tests/inferers/test_zarr_avg_merger.py", line 430, in test_deprecated_count_compressor_warning
    ZarrAvgMerger(merged_shape=TENSOR_4x4.shape, count_compressor="LZ4")
  File "/home/runner/work/MONAI/MONAI/monai/utils/module.py", line 477, in _wrapper
    return call_obj(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/utils/deprecate_utils.py", line 223, in _wrapper
    return func(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/utils/deprecate_utils.py", line 223, in _wrapper
    return func(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/utils/deprecate_utils.py", line 223, in _wrapper
    return func(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/inferers/merger.py", line 365, in __init__
    self.counts = zarr.zeros(
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/creation.py", line 338, in zeros
    return create(shape=shape, fill_value=0, **kwargs)
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/creation.py", line 209, in create
    init_array(
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/storage.py", line 455, in init_array
    _init_array_metadata(
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/storage.py", line 564, in _init_array_metadata
    raise BadCompressorError(compressor) from e
zarr.errors.BadCompressorError: bad compressor; expected Codec object, found 'LZ4'

======================================================================
ERROR: test_deprecated_value_compressor_warning (tests.inferers.test_zarr_avg_merger.ZarrAvgMergerTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/storage.py", line 562, in _init_array_metadata
    compressor_config = compressor.get_config()
AttributeError: 'str' object has no attribute 'get_config'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/runner/work/MONAI/MONAI/tests/inferers/test_zarr_avg_merger.py", line 421, in test_deprecated_value_compressor_warning
    ZarrAvgMerger(merged_shape=TENSOR_4x4.shape, value_compressor="LZ4")
  File "/home/runner/work/MONAI/MONAI/monai/utils/module.py", line 477, in _wrapper
    return call_obj(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/utils/deprecate_utils.py", line 223, in _wrapper
    return func(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/utils/deprecate_utils.py", line 223, in _wrapper
    return func(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/utils/deprecate_utils.py", line 223, in _wrapper
    return func(*args, **kwargs)
  File "/home/runner/work/MONAI/MONAI/monai/inferers/merger.py", line 357, in __init__
    self.values = zarr.zeros(
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/creation.py", line 338, in zeros
    return create(shape=shape, fill_value=0, **kwargs)
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/creation.py", line 209, in create
    init_array(
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/storage.py", line 455, in init_array
    _init_array_metadata(
  File "/opt/hostedtoolcache/Python/3.9.23/x64/lib/python3.9/site-packages/zarr/storage.py", line 564, in _init_array_metadata
    raise BadCompressorError(compressor) from e
zarr.errors.BadCompressorError: bad compressor; expected Codec object, found 'LZ4'

@kolasaniv1996 kolasaniv1996 force-pushed the fix-zarr-compressor branch 2 times, most recently from 93d2146 to 389b300 Compare June 12, 2025 17:37
kolasaniv1996 and others added 2 commits June 13, 2025 00:41
Signed-off-by: vivek kolasani <[email protected]>
Signed-off-by: YunLiu <[email protected]>
@KumoLiu
Copy link
Contributor

KumoLiu commented Jun 13, 2025

Hi @kolasaniv1996 and @ericspod, I try to fix the ci issue in this commit: cac8923
Let me know if you have any further concern.

BTW @kolasaniv1996, could you please follow this guide to fix the DCO issue: https://github.com/Project-MONAI/MONAI/pull/8477/checks?check_run_id=44029533563? Thanks!

KumoLiu and others added 3 commits June 13, 2025 15:54
@KumoLiu
Copy link
Contributor

KumoLiu commented Jun 13, 2025

/build

@KumoLiu KumoLiu enabled auto-merge (squash) June 13, 2025 15:39
@KumoLiu KumoLiu merged commit 3d6021a into Project-MONAI:dev Jun 13, 2025
25 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