Skip to content

Add support for Zarr-Python v3 and Zarr format v3#530

Open
FedeMPouzols wants to merge 28 commits intomainfrom
355-zarr-python-v3-and-zarr-v3
Open

Add support for Zarr-Python v3 and Zarr format v3#530
FedeMPouzols wants to merge 28 commits intomainfrom
355-zarr-python-v3-and-zarr-v3

Conversation

@FedeMPouzols
Copy link
Copy Markdown
Collaborator

Fixes #355

By default Zarr v3 will be used. Note this includes a "ZARR_VERSION" in xradio._utils.zarr.config that the user can change to force v2.

@FedeMPouzols FedeMPouzols linked an issue Jan 22, 2026 that may be closed by this pull request
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@FedeMPouzols
Copy link
Copy Markdown
Collaborator Author

@dmehring . @Jan-Willem : I was puzzled by this failure in test_image / casacore_to_xds_to_casacore.test_metadata, that occurs only in the casatools job:

tests/unit/image/test_image.py:1099: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
tests/unit/image/test_image.py:131: in dict_equality
    self.dict_equality(
tests/unit/image/test_image.py:149: in dict_equality
    self.assertEqual(
E   AssertionError: 'sky' != 'Intensity'
E   - sky
E   + Intensity
E    : got[imageinfo][imagetype] != expected[imageinfo][imagetype]:
E   sky vs
E   Intensity

But now I see that the same failure happened already in other PRs merged recently:
https://github.com/casangi/xradio/actions/runs/21250448726/job/61150355293
https://github.com/casangi/xradio/actions/runs/21227589404/job/61078515210 (the issue seems to have started here)

I've tried to add a fix for it in this commit and it seems to work. Does that sound reasonable, or might there be side-effects/related issues?

@r-xue
Copy link
Copy Markdown
Collaborator

r-xue commented Jan 24, 2026

Hi @FedeMPouzols,

The underlying issue might be related to the behavior of imagetype values returned by python-casacore's image.info(). I noticed this previously, and while the casatools wrapper attempts to partially mimic its behavior, this test reveals further problems.

Here is a small test script to illustrate the behavior of the python-casacore image and table modules regarding the imageinfo keyword and imagetype.

try:
    from casacore import images, tables
except ImportError:
    import xradio._utils._casacore.casacore_from_casatools as images
    import xradio._utils._casacore.casacore_from_casatools as tables
# Note: This is a python-casacore-only environment.

In [27]: tb=tables.table('out.im',readonly=False)
    ...: tb.putkeyword('imageinfo',{'imagetype': 'sky', 'objectname': 'ngc1234'})
    ...: tb.close()
    ...: 
    ...: ia=images.image('out.im')
    ...: print(ia.info()['imageinfo'])
    ...: tb=tables.table('out.im')
    ...: print(tb.getkeyword('imageinfo'))
Successful read/write open of default-locked table out.im: 1 columns, 1 rows
{'imagetype': 'Intensity', 'objectname': 'ngc1234'}
Successful readonly open of default-locked table out.im: 1 columns, 1 rows
{'imagetype': 'sky', 'objectname': 'ngc1234'}

In [28]: tb=tables.table('out.im',readonly=False)
    ...: tb.putkeyword('imageinfo',{'imagetype': 'beam', 'objectname': 'ngc1234'})
    ...: tb.close()
    ...: 
    ...: ia=images.image('out.im')
    ...: print(ia.info()['imageinfo'])
    ...: tb=tables.table('out.im')
    ...: print(tb.getkeyword('imageinfo'))
Successful read/write open of default-locked table out.im: 1 columns, 1 rows
{'imagetype': 'Beam', 'objectname': 'ngc1234'}
Successful readonly open of default-locked table out.im: 1 columns, 1 rows
{'imagetype': 'beam', 'objectname': 'ngc1234'}

You can see that image.info() from python-casacore fails to recover the "desired" 'sky' imagetype, even after we explicitly wrote it into the table keyword imageinfo. Instead, it defaults to 'Intensity' unless we write 'beam' (lower-case), in which case it correctly returns 'Beam'.

I suspect this is due to the internal enum enforcement of imagetype in the casacore C++ code:
https://casacore.github.io/casacore/classcasacore_1_1ImageInfo.html
However, I haven't dug into the C++ code to verify this.

One implication of this "feature" is that... if we rely on casacore's tb.putkeywords('imageinfo') to write a "sky" image type and later use image.info to retrieve it, the CASA-images -> xds -> CASA-images round trip might not work as expected.

The reason the current tests work for the python-casacore backend is likely coincidental: either the input/output files don't have "imageinfo" keywords, or the written imagetype value ("sky") fails the enum requirement, causing info['imageinfo']['imagetype'] to fall back to the default 'Intensity'. From that perspective, the test might be passing just by "luck."

On the other hand, the image.info offered by the casatools wrapper was a patch to mimic python-casacore's imageinfo. However, under the hood, it tries to read from the imageinfo table keywords directly:

Therefore, it "successfully" retrieves the "sky" imagetype from the test output images keyword (which may have been added during the round trip). This creates a discrepancy with the initial test input CASA images, which lack the "imageinfo" keyword and default to "Intensity".

The bigger question (potentially to @dmehring) is: How do we embed/retrieve imagetype in a reliable way that works for both backends? Do we need to ignore image.info() and instead directly retrieve the imageinfo keyword from the table whenever we need the imagetype?

@FedeMPouzols FedeMPouzols marked this pull request as ready for review February 1, 2026 14:21
@FedeMPouzols
Copy link
Copy Markdown
Collaborator Author

FedeMPouzols commented Feb 4, 2026

The integration tests are timing out because the unit test astroVIPER/test_feather hangs. Fiddling a bit with this test I can see that it triggers at least one ImportError in xradio/image/_util/_zarr/zarr_low_level.py, function create_data_variable_meta_data(). The missing import n5 is no longer available in Zarr v3. This function create_data_variable_meta_data() is apparently not exercised in XRadio but it is used in astroVIPER/distributed/imaging/feather and cube_imaging_niter0.

I've cancelled the last re-run of the integration test job, as it was already stuck in test_feather, until we find a solution for the n5 issue (and for now I also see direct writing of the attributes json file in that function which will need to be changed / ideally should be avoided).

@r-xue
Copy link
Copy Markdown
Collaborator

r-xue commented Mar 31, 2026

Hi @FedeMPouzols and @Jan-Willem — just checking in on the plan for #355 and this PR.

I'm drafting #557 to introduce Zarr sharding, which might help mitigate potential edge cases where chunking overwhelms the Lustre MDS/OSTs. Since #557 is using this PR/branch as the base, I want to make sure it aligns with your timeline.

@FedeMPouzols
Copy link
Copy Markdown
Collaborator Author

@r-xue : I think this branch was in principle planned to be merged around the time the PR was opened. Zarr v3 was working well and the only issue remaining is some warnings related to string types (as discussed in #355). It was decided to not wait for that to be fixed, as it has been an standing issue for a few months and it looks like a low priority at the moment for Zarr developers.
But we found that some imaging related VIPER functions/tests need changes to be Zarr v3-compatible. I believe this is waiting for other ongoing changes in the VIPER imaging functionality/demonstrator.
Jan-Willem can clarify better the more general plans re Zarr v3 etc.

@FedeMPouzols
Copy link
Copy Markdown
Collaborator Author

I've merged main into this branch. Also, Zarr 3.1.6 was released a few days ago (it comes with a couple of sharding related fixes). This will trigger the CI tests again and I'd expect their results to be still the same (perhaps with some new tests/changes in the VIPER tests).

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 4, 2026

Codecov Report

❌ Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...radio/measurement_set/_utils/_msv2/_tables/read.py 75.00% 2 Missing ⚠️
src/xradio/image/_util/_zarr/xds_to_zarr.py 66.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

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.

Zarr Python v3 and Zarr v3

3 participants