Skip to content
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

Fill value fixes for V3 #1

Merged
merged 5 commits into from
Oct 9, 2024

Conversation

rabernat
Copy link

@rabernat rabernat commented Oct 9, 2024

This gets many of the backend tests passing against V3. Still some work to go.

Main issues addressed:

  • only use fill_value for signaling missing data in V2 data via use_zarr_fill_value_as_mask parameter
  • add new Numpy2StringDTypeCoder to deal with StringDType data coming out of Zarr

Summary of failures (many V2 due to zarr-developers/zarr-python#2315)

$ pytest -v xarray/tests/test_backends.py::TestZarrDictStore
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_roundtrip_object_dtype[2] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_roundtrip_empty_vlen_string_array[2] - AssertionError: assert dtype('<U') == dtype('<U1')
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_write_persistence_modes[2-None] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_write_persistence_modes[2-group1] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_check_encoding_is_consistent_after_append[2] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_append_with_new_variable[2] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_append_with_append_dim_no_overwrite[2] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_to_zarr_append_compute_false_roundtrip[2] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_roundtrip_bytes_with_fill_value[3] - TypeError: Invalid attribute in Dataset.attrs.
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_roundtrip_endian[3] - KeyError: '>i2'
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_default_fill_value[3] - TypeError: must be real number, not str
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_explicitly_omit_fill_value_via_encoding_kwarg[3] - ValueError: unexpected encoding parameters for zarr backend:  ['_FillValue']
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_explicitly_omit_fill_value_in_coord_via_encoding_kwarg[3] - ValueError: unexpected encoding parameters for zarr backend:  ['_FillValue']
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_append_string_length_mismatch_raises[3-U] - Failed: DID NOT RAISE <class 'ValueError'>
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_append_string_length_mismatch_raises[3-S] - Failed: DID NOT RAISE <class 'ValueError'>
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_check_encoding_is_consistent_after_append[3] - AttributeError: 'list' object has no attribute 'get_config'

@rabernat
Copy link
Author

rabernat commented Oct 9, 2024

In 34c4c24 I added some custom encoding / decoding for _FillValue which mostly makes things work in V3.

Latest list of failing tests

FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_roundtrip_object_dtype[2] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_roundtrip_empty_vlen_string_array[2] - AssertionError: assert dtype('<U') == dtype('<U1')
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_write_persistence_modes[2-None] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_write_persistence_modes[2-group1] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_check_encoding_is_consistent_after_append[2] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_append_with_new_variable[2] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_append_with_append_dim_no_overwrite[2] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_to_zarr_append_compute_false_roundtrip[2] - ValueError: When changing to a smaller dtype, its size must be a divisor of the size of original dtype
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_roundtrip_endian[3] - KeyError: '>i2'
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_explicitly_omit_fill_value_via_encoding_kwarg[3] - ValueError: unexpected encoding parameters for zarr backend:  ['_FillValue']
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_explicitly_omit_fill_value_in_coord_via_encoding_kwarg[3] - ValueError: unexpected encoding parameters for zarr backend:  ['_FillValue']
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_append_string_length_mismatch_raises[3-U] - Failed: DID NOT RAISE <class 'ValueError'>
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_append_string_length_mismatch_raises[3-S] - Failed: DID NOT RAISE <class 'ValueError'>
FAILED xarray/tests/test_backends.py::TestZarrDictStore::test_check_encoding_is_consistent_after_append[3] - AttributeError: 'list' object has no attribute 'get_config'

The roundtrip_endian is related to zarr-developers/zarr-python#2324

@TomAugspurger
Copy link
Owner

Looks good, thanks. There's a merge conflict now that I added the zarr_format / zarr_version deprecation to this branch. LMK if you want me to resolve the conflicts.

@TomAugspurger
Copy link
Owner

Actually I'm fixing some merge conflicts in other spots right now and will handle this quick.

@TomAugspurger TomAugspurger merged commit e6e2066 into TomAugspurger:fix/zarr-v3 Oct 9, 2024
1 check 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.

2 participants