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

Add ParquetFileArrowReader::try_new #1782

Merged
merged 3 commits into from
Jun 3, 2022

Conversation

tustvold
Copy link
Contributor

@tustvold tustvold commented Jun 3, 2022

Which issue does this PR close?

Closes #.

Rationale for this change

There is an extremely common pattern of creating a SerializedFileReader only to pass it to the constructor to ParquetFileArrowReader. Not only is this cumbersome, but the Arc part is a little bit surprising especially to new users.

What changes are included in this PR?

Adds try_new and try_new_with_options methods to ParquetFileArrowReader to simplify this common case

Are there any user-facing changes?

No

@github-actions github-actions bot added the parquet Changes to the parquet crate label Jun 3, 2022
@tustvold tustvold force-pushed the parquet-file-arrow-reader-try-new branch from 62aed2f to 6a55ce7 Compare June 3, 2022 13:10
@alamb alamb changed the title Add ParquetFileArrowReader::try_new Add ParquetFileArrowReader::try_new Jun 3, 2022
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Looks much nicer to me 👨‍🍳 👌

parquet/src/arrow/mod.rs Show resolved Hide resolved
parquet/src/arrow/arrow_reader.rs Show resolved Hide resolved
@tustvold tustvold force-pushed the parquet-file-arrow-reader-try-new branch from 97dde6e to d2b6bf7 Compare June 3, 2022 21:14
@codecov-commenter
Copy link

Codecov Report

Merging #1782 (02e81ef) into master (eb706b7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 02e81ef differs from pull request most recent head d2b6bf7. Consider uploading reports for the commit d2b6bf7 to get more accurate results

@@           Coverage Diff           @@
##           master    #1782   +/-   ##
=======================================
  Coverage   83.39%   83.39%           
=======================================
  Files         198      198           
  Lines       56181    56171   -10     
=======================================
- Hits        46850    46844    -6     
+ Misses       9331     9327    -4     
Impacted Files Coverage Δ
parquet/src/arrow/mod.rs 44.44% <ø> (ø)
parquet/src/arrow/arrow_reader.rs 96.87% <100.00%> (+0.74%) ⬆️
parquet/src/arrow/arrow_writer.rs 97.76% <100.00%> (-0.02%) ⬇️
parquet/src/arrow/schema.rs 96.81% <100.00%> (-0.01%) ⬇️
parquet_derive/src/parquet_field.rs 65.53% <0.00%> (-0.46%) ⬇️
parquet/src/encodings/encoding.rs 93.65% <0.00%> (+0.19%) ⬆️
arrow/src/datatypes/datatype.rs 65.79% <0.00%> (+0.37%) ⬆️

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 eb706b7...d2b6bf7. Read the comment docs.

@tustvold tustvold merged commit 940b5b5 into apache:master Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parquet Changes to the parquet crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants