Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

add bytes dtype#3559

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
d-v-b wants to merge2 commits intozarr-developers:main
base:main
Choose a base branch
Loading
fromd-v-b:chore/bytes-dtype
Draft

Conversation

@d-v-b
Copy link
Contributor

@d-v-bd-v-b commentedOct 28, 2025
edited
Loading

This PR adds aBytes dtype that is nearly identical to the existingVariableLengthBytes dtype save a few differences:

  • the V3 JSON form is"bytes" instead of"variable_length_bytes"
  • The fill value representation is an array of ints, instead of a base64-encoded string
  • Bytes is consistent witha published spec, instead of not being described by a spec.

The latter point is the most important.

BecauseBytes is nearly identical toVariableLengthBytes, it could be a drop-in replacement forVariableLengthBytes, save for the JSON fill value encoding differences between the two codecs, which raises two questions:

  • is base64 encoding a bytestring a better or worse JSON serialization than representing the same bytestring as an array of integers?
  • could (or should) we amend the bytes data type spec to recommend reading (or reading and writing) both fill value encodings? If so, thebytes data type and thevariable_length_bytes data types could be fused completely.

Since zarr python 2.x was saving bytes fill values as base64-encoded strings, there's a bit of inertia there. Would also be good to hear thoughts from other implementers (@LDeakin ,@jbms,@manzt )

@github-actionsgithub-actionsbot added the needs release notesAutomatically applied to PRs which haven't added release notes labelOct 28, 2025
@codecov
Copy link

codecovbot commentedOct 28, 2025
edited
Loading

Codecov Report

❌ Patch coverage is46.47887% with38 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.76%. Comparing base (eb1b6e8) to head (702f0ae).

Files with missing linesPatch %Lines
src/zarr/core/dtype/npy/bytes.py46.37%37 Missing⚠️
src/zarr/core/dtype/npy/common.py50.00%1 Missing⚠️
Additional details and impacted files
@@            Coverage Diff             @@##             main    #3559      +/-   ##==========================================- Coverage   61.87%   61.76%   -0.11%==========================================  Files          85       85                Lines       10134    10202      +68     ==========================================+ Hits         6270     6301      +31- Misses       3864     3901      +37
Files with missing linesCoverage Δ
src/zarr/core/dtype/npy/common.py61.53% <50.00%> (-0.19%)⬇️
src/zarr/core/dtype/npy/bytes.py51.12% <46.37%> (-1.88%)⬇️
🚀 New features to boost your workflow:
  • ❄️Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

needs release notesAutomatically applied to PRs which haven't added release notesspec compliance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@d-v-b

[8]ページ先頭

©2009-2025 Movatter.jp