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.

Sign up for GitHub

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

Jump to bottom

Improve ParquetRecordBatchStreamBuilder docs / examples #6948

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve ParquetRecordBatchStreamBuilder docs / examples #6948

wants to merge 1 commit into from

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Jan 6, 2025

Which issue does this PR close?

Rationale for this change

While working on the code I found:

  1. The examples are in the module docs, not on the struct used to construct async readers
  2. The example had both basic usage as well as more advanced usage (setting options / projection selection) which I think it makes harder to understand

What changes are included in this PR?

  1. Move example to ParquetRecordBatchStreamBuilder::new
  2. Split into 2 examples: one for basic usage and one for more advanced usage

Are there any user-facing changes?

Docs only, no functional chagne

@github-actions github-actions bot added the parquet Changes to the parquet crate label Jan 6, 2025
alamb
alamb commented Jan 6, 2025
View reviewed changes
parquet/src/arrow/async_reader/mod.rs
//! );
//! # }
//! ```
//! See example on [`ParquetRecordBatchStreamBuilder`]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved examples to ParquetRecordBatchStreamBuilder

parquet/src/arrow/async_reader/mod.rs
/// assert_batches_eq(
/// &results,
/// &[
/// "+----+----------+-------------+--------------+---------+------------+-----------+------------+------------------+------------+---------------------+",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

made the first example select all columns to make it simpler to understand

parquet/src/arrow/async_reader/mod.rs
///
/// ```
/// # use std::fs::metadata;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the existing example is less compelling in my mind than what was on the module level docs so I remove the current one)

parquet/src/arrow/async_reader/mod.rs
/// # }
/// ```
///
/// # Example configuring options and reading metadata
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a more advanced example with projection pushdown

@alamb alamb added the documentation Improvements or additions to documentation label Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
documentation Improvements or additions to documentation parquet Changes to the parquet crate
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@alamb