Skip to content

Normalize kerchunk refs format to true json before comparisons #680

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.

Sign up for GitHub

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

Jump to bottom
Merged
TomNicholas merged 11 commits into from
Jul 16, 2025
Merged

Normalize kerchunk refs format to true json before comparisons #680

TomNicholas merged 11 commits into from
Jul 16, 2025

Conversation

@TomNicholas
Copy link
Member

@TomNicholas TomNicholas commented Jul 16, 2025 •
edited
Loading

@TomNicholas TomNicholas added the Kerchunk Relating to the kerchunk library / specification itself label Jul 16, 2025
@TomNicholas TomNicholas mentioned this pull request Jul 16, 2025
Closed
@TomNicholas TomNicholas changed the title Normalize kerchunk reference to true json before comparisons Normalize kerchunk refs format to true json before comparisons Jul 16, 2025
@TomNicholas TomNicholas requested a review from maxrjones July 16, 2025 14:53
@TomNicholas
Copy link
Member Author

TomNicholas commented Jul 16, 2025

Everything fails because everything is broken on develop thanks to zarr's recent release, not because this PR breaks things. Though it's difficult to be certain until we fix it (which requires #677, which requires this PR). But the tests altered in this PR do pass.

@TomNicholas TomNicholas mentioned this pull request Jul 16, 2025
Merged
7 tasks
@maxrjones
Copy link
Member

maxrjones commented Jul 16, 2025

Everything fails because everything is broken on develop thanks to zarr's recent release, not because this PR breaks things. Though it's difficult to be certain until we fix it (which requires #677, which requires this PR). But the tests altered in this PR do pass.

The tests pass now that Zarr is pinned, but I think it's worth figuring out the typing issues since they can catch problems not captured in our tests.

@TomNicholas
Copy link
Member Author

TomNicholas commented Jul 16, 2025

I just fixed the typing issues locally - they are unsurprising because I had not told the type hints that this isn't just any old JSON structure, it's specific to Kerchunk.

@TomNicholas
Copy link
Member Author

TomNicholas commented Jul 16, 2025

(Also very good idea to pin the dependencies)

@TomNicholas TomNicholas merged commit 2e652c3 into zarr-developers:develop Jul 16, 2025
11 checks passed
@TomNicholas TomNicholas deleted the kerchunk_to_true_json branch July 16, 2025 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@maxrjones maxrjones maxrjones approved these changes

Assignees

No one assigned

Labels

Kerchunk Relating to the kerchunk library / specification itself

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Breaking change to Kerchunk references spec output

2 participants

@TomNicholas @maxrjones