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

Added compression options/levels for GZIP and BROTLI codecs. #132

Merged
merged 1 commit into from
Apr 28, 2022

Conversation

TurnOfACard
Copy link
Contributor

  • Sets the default GZIP compression == 6, as this is the default within miniz_oxide.
  • brotli quality is kept the same, as no default was listed with the brotli rust crate.

This is a breaking API change. However, it is an extension of the previous breaking change to zstd levels.

* Sets the default GZIP compression == 6, as this is the default within miniz_oxide.
* brotli quality is kept the same, as no default was listed with the brotli rust crate.

This is a breaking API change, however it is an extension of the previous breaking change to zstd levels.
@TurnOfACard
Copy link
Contributor Author

TurnOfACard commented Apr 28, 2022

I left the other compression types alone:

  • snappy doesn't appear to have compression levels
  • lz4 and lz4_flex can't take the same compression levels
  • lzo is unimplemented, and it could potentially be removed from CompressionOptions?

Finally, in arrow2 the compression levels should be re-exported

@codecov-commenter
Copy link

Codecov Report

Merging #132 (d509434) into main (95826dd) will increase coverage by 0.08%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##             main     #132      +/-   ##
==========================================
+ Coverage   74.27%   74.35%   +0.08%     
==========================================
  Files          78       78              
  Lines        3553     3576      +23     
==========================================
+ Hits         2639     2659      +20     
- Misses        914      917       +3     
Impacted Files Coverage Δ
src/parquet_bridge.rs 70.13% <84.00%> (+1.32%) ⬆️
src/compression.rs 84.61% <100.00%> (+1.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95826dd...d509434. Read the comment docs.

/// Compression levels must be valid (i.e. be acceptable for [`zstd::compression_level_range`])
#[cfg(feature = "zstd")]
pub fn try_new(level: i32) -> Result<Self, Error> {
let compression_range = zstd::compression_level_range();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this call so the trait could be used across the different compression types

@jorgecarleitao jorgecarleitao changed the title Add compression options/levels for GZIP and BROTLI codecs. Added compression options/levels for GZIP and BROTLI codecs. Apr 28, 2022
@jorgecarleitao jorgecarleitao added the feature A new feature label Apr 28, 2022
@jorgecarleitao jorgecarleitao merged commit 3256d28 into jorgecarleitao:main Apr 28, 2022
@jorgecarleitao
Copy link
Owner

Awesome PR. Thanks a lot @TurnOfACard !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants