Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Convert NCzarr meta-data to use only Zarr attributes #2936

Merged
merged 3 commits into from
Jul 8, 2024

Conversation

DennisHeimbigner
Copy link
Collaborator

@DennisHeimbigner DennisHeimbigner commented Jun 20, 2024

As discussed in a netcdf meeting, convert NCZarr V2 to store all netcdf-4 specific info as attributes. This improves interoperability with other Zarr implementations by no longer using non-standard keys. The price to be paid is that lazy attribute reading cannot be supported.

Other Changes

  • Remove support for older NCZarr formats.
  • Update anonymous dimension naming
  • Begin the process of fixing the -Wconversion and -Wsign-compare warnings in libnczarr, nczarr_test, and v3_nczarr_test.
  • Update docs/nczarr.md
  • Rebuild using the .y and .l files

Addendum

As suggested by Ward, I ensured that this PR supports read backward compatibility with old key format. This addition also adds a test case for this.

Misc. Other Changes

  • Remove some unused code
  • Cleanup json error handling
  • Fix some more unsigned/signed conversions warning

As discussed in a netcdf meeting, convert NCZarr V2 to store all netcdf-4 specific info as attributes. This improves interoperability with other Zarr implementations by no longer using non-standard keys.

## Other Changes
* Remove support for older NCZarr formats.
* Update anonymous dimension naming
* Begin the process of fixing the -Wconversion and -Wsign-compare warnings in libnczarr, nczarr_test, and v3_nczarr_test.
* Update docs/nczarr.md
* Rebuild using the .y and .l files
@DennisHeimbigner DennisHeimbigner requested a review from WardF as a code owner June 20, 2024 00:10
WardF
WardF previously approved these changes Jun 21, 2024
@WardF
Copy link
Member

WardF commented Jun 21, 2024

This looks good, but my main concern is Remove support for older NCZarr formats.. I know they aren't in particularly wide use yet, but is this write support, or read support as well?

@DennisHeimbigner
Copy link
Collaborator Author

You are right. I should ensure backward read compatibility.
Will take a day or two.

As suggested by Ward, I ensured that this PR supports
read backward compatibility with old key format.
This addition also adds a test case for this.

## Misc. Other Changes
* Remove some unused code
* Cleanup json error handling
* Fix some more unsigned/signed conversions warning
@DennisHeimbigner
Copy link
Collaborator Author

This appears to ready to merge.

@WardF WardF self-assigned this Jul 8, 2024
@WardF WardF merged commit 18a98c4 into Unidata:main Jul 8, 2024
106 checks passed
@DennisHeimbigner DennisHeimbigner deleted the v2new.dmh branch August 14, 2024 02:11
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants