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

base64 encode fill value for some dtypes with zarr_format=2 #2286

Conversation

TomAugspurger
Copy link
Contributor

@TomAugspurger TomAugspurger commented Oct 1, 2024

Closes #2282

Needs more tests.

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/tutorial.rst
  • Changes documented in docs/release.rst
  • GitHub Actions have all passed
  • Test coverage is 100% (Codecov passes)

@TomAugspurger TomAugspurger marked this pull request as ready for review October 2, 2024 19:40
@TomAugspurger
Copy link
Contributor Author

TomAugspurger commented Oct 2, 2024

This should be ready to go (the CI failure is from appveyor).

@TomAugspurger TomAugspurger changed the title [WIP]: base64 encode fill value for some dtypes with zarr_format=2 base64 encode fill value for some dtypes with zarr_format=2 Oct 2, 2024
@d-v-b
Copy link
Contributor

d-v-b commented Oct 2, 2024

were we not doing this in the v2 codebase? I couldn't find code for it.

@TomAugspurger
Copy link
Contributor Author

I found it at

def decode_fill_value(cls, v: Any, dtype: np.dtype, object_codec: Any = None) -> Any:
(and borrowed the implementation).

src/zarr/core/metadata/v2.py Outdated Show resolved Hide resolved
@jakirkham
Copy link
Member

Thanks Tom! 🙏

JFYI we have a Base64 codec in Numcodecs, perhaps it is worth using to align on behavior

@TomAugspurger
Copy link
Contributor Author

Thanks John. I think this version (rather than numcodec's) is OK. It ends up using the same actual implementation of base64.standard_b64encode but it's slightly more direct with the calls directly in the serialization code in this PR.

@TomAugspurger TomAugspurger merged commit cc50b41 into zarr-developers:v3 Oct 7, 2024
20 checks passed
@TomAugspurger TomAugspurger deleted the tom/fix/v2-encode-decode-fill-value branch October 7, 2024 19:04
@jhamman jhamman added the V3 Affects the v3 branch label Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V3 Affects the v3 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix fill_value encoding for fixed-width string / binary v2 array
5 participants