-
Notifications
You must be signed in to change notification settings - Fork 9
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
Added zstandard compression tooling, added to bytes codec for JSON serialization #438
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do the NPY_DTYPE and NUMPY codecs also need updating here?
…e into zst-bytes-compression
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #438 +/- ##
=======================================
Coverage 98.49% 98.50%
=======================================
Files 36 37 +1
Lines 2131 2143 +12
=======================================
+ Hits 2099 2111 +12
Misses 32 32 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great @atravitz! I gave this a full review, and generalized test_legacy_bytes_uncompressed
to apply to all CustomJSONCodingTest
subclasses.
Can you add a news entry? After that I think this is good to go!
@IAlibay they do not! These convert to bytes, and then the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of things, otherwise lgtm
Co-authored-by: Irfan Alibay <[email protected]>
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
Closes #321.
Tips
Since this will create a commit, it is best to make this comment when you are finished with your work.
Checklist
news
entryDevelopers certificate of origin