Skip to content

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

Merged
TomAugspurger merged 7 commits into
zarr-developers:v3from
TomAugspurger:tom/fix/v2-encode-decode-fill-value
Oct 7, 2024
Merged

base64 encode fill value for some dtypes with zarr_format=2#2286
TomAugspurger merged 7 commits into
zarr-developers:v3from
TomAugspurger:tom/fix/v2-encode-decode-fill-value

Conversation

@TomAugspurger

@TomAugspurger TomAugspurger commented Oct 1, 2024

Copy link
Copy Markdown
Contributor

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)

Comment thread src/zarr/core/metadata/v2.py
@TomAugspurger
TomAugspurger marked this pull request as ready for review October 2, 2024 19:40
@TomAugspurger

TomAugspurger commented Oct 2, 2024

Copy link
Copy Markdown
Contributor Author

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

d-v-b commented Oct 2, 2024

Copy link
Copy Markdown
Contributor

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

@TomAugspurger

Copy link
Copy Markdown
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).

Comment thread src/zarr/core/metadata/v2.py Outdated
@jakirkham

Copy link
Copy Markdown
Member

Thanks Tom! 🙏

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

@TomAugspurger

Copy link
Copy Markdown
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
@TomAugspurger
TomAugspurger deleted the tom/fix/v2-encode-decode-fill-value branch October 7, 2024 19:04
@jhamman jhamman added the V3 label Oct 11, 2024
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.

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

5 participants