docs(parquet): move async parquet example into ArrowReaderBuilder docs #9167

vigneshsiva11 · 2026-01-14T07:40:42Z

Rationale for this change

The async parquet reader example under parquet/examples duplicated functionality
that is already exposed through the public ArrowReaderBuilder::try_new API.

Moving this example into the API documentation improves discoverability for users
while avoiding the need to maintain duplicate standalone examples.

What changes are included in this PR?

Added an async parquet reading example to the rustdoc comments for
ArrowReaderBuilder::try_new
Removed the redundant async_read_parquet.rs example file from
parquet/examples

Are these changes tested?

Yes. Existing tests were run locally using:

cargo test -p parquet

Copilot

Pull request overview

This PR aims to move an async parquet reading example from a standalone example file into the API documentation for ArrowReaderBuilder::try_new. However, there's a fundamental mismatch: the deleted file demonstrated async parquet reading using ParquetRecordBatchStreamBuilder and tokio::fs::File, while the newly added documentation example uses synchronous APIs (std::fs::File and ParquetRecordBatchReaderBuilder).

Changes:

Removed the async_read_parquet.rs example file that demonstrated async reading with ParquetRecordBatchStreamBuilder
Added a synchronous example to ParquetRecordBatchReaderBuilder::try_new documentation that is incorrectly wrapped in an async function

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
parquet/examples/async_read_parquet.rs	Removed standalone async example demonstrating `ParquetRecordBatchStreamBuilder` with async file I/O
parquet/src/arrow/arrow_reader/mod.rs	Added synchronous example incorrectly labeled as async to `try_new` documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-14T07:43:02Z

parquet/src/arrow/arrow_reader/mod.rs

+
+
+    /// # Example
+    /// ```rust
+    /// use parquet::arrow::arrow_reader::ArrowReaderBuilder;
+    /// # async fn example() -> parquet::errors::Result<()> {
+    /// let file = std::fs::File::open("data.parquet")?;
+    /// let builder = ArrowReaderBuilder::try_new(file)?;
+    /// let mut reader = builder.build()?;
+    /// # Ok(())
+    /// # }
+    /// ```
+


Extra blank lines before and after the example section. Standard Rust documentation style typically uses a single blank line between doc comment sections.

Copilot · 2026-01-14T07:43:02Z

parquet/src/arrow/arrow_reader/mod.rs

+    /// # Example
+    /// ```rust
+    /// use parquet::arrow::arrow_reader::ArrowReaderBuilder;
+    /// # async fn example() -> parquet::errors::Result<()> {
+    /// let file = std::fs::File::open("data.parquet")?;
+    /// let builder = ArrowReaderBuilder::try_new(file)?;
+    /// let mut reader = builder.build()?;
+    /// # Ok(())
+    /// # }
+    /// ```


This example is marked as async (wrapped in async fn example()) but demonstrates synchronous file reading using std::fs::File. The deleted async_read_parquet.rs example actually demonstrated async reading using tokio::fs::File and ParquetRecordBatchStreamBuilder.

If the intent is to provide an async example for ArrowReaderBuilder::try_new, it should use the async API (ParquetRecordBatchStreamBuilder) instead of the synchronous API (ParquetRecordBatchReaderBuilder). Alternatively, if this is meant to be a synchronous example, the async fn example() wrapper should be removed and replaced with a regular function.

Copilot · 2026-01-14T07:43:03Z

parquet/src/arrow/arrow_reader/mod.rs

+    /// # async fn example() -> parquet::errors::Result<()> {
+    /// let file = std::fs::File::open("data.parquet")?;
+    /// let builder = ArrowReaderBuilder::try_new(file)?;
+    /// let mut reader = builder.build()?;


The example creates a reader but never demonstrates using it to read data. Consider adding code that actually reads from the reader (e.g., iterating over batches) to provide a more complete demonstration of the API's functionality.

Suggested change

/// let mut reader = builder.build()?;

/// let mut reader = builder.build()?;

///

/// // Read all record batches from the reader

/// while let Some(batch) = reader.next().transpose()? {

/// println!("Read {} rows", batch.num_rows());

/// }

Copilot · 2026-01-14T07:43:03Z

parquet/src/arrow/arrow_reader/mod.rs

+
+
+    /// # Example
+    /// ```rust


The hardcoded file path "data.parquet" will cause this example to fail when run via cargo test --doc. Examples in documentation should either use files that are known to exist (like those in the test data directory), or be marked with no_run to prevent execution during doc tests.

Suggested change

/// ```rust

/// ```rust,no_run

vigneshsiva11 · 2026-01-14T08:06:49Z

Thanks for the feedback!

I’ve updated the example to be fully synchronous, added a minimal read loop,
marked it as no_run, and aligned it with standard Rustdoc conventions.
All parquet tests and doctests pass locally.

Please let me know if this looks good now.

alamb

Thank you for the contribution @vigneshsiva11

alamb · 2026-01-14T22:52:39Z

parquet/src/arrow/arrow_reader/mod.rs

+
+    /// # Example
+    /// ```rust
+    /// use parquet::arrow::arrow_reader::ArrowReaderBuilder;


this is an example for a different structure, right?

The doc example is on ParquetRecordBatchReaderBuilder but the example is using ArrowReaderBuilder 🤔

Hi @alamb, I’ve pushed an update converting the example into a doctest as suggested.
All checks pass locally (fmt, clippy, test).
Please let me know if you’d like any changes. Thanks!

…uilder

alamb

Thank you for the contribution @vigneshsiva11

parquet/src/arrow/arrow_reader/mod.rs

alamb · 2026-01-17T15:46:58Z

parquet/src/arrow/arrow_reader/mod.rs


 impl<T: ChunkReader + 'static> ParquetRecordBatchReaderBuilder<T> {
    /// Create a new [`ParquetRecordBatchReaderBuilder`]
-    ///


I think this change means the old example isn't run

alamb · 2026-01-17T15:47:35Z

@vigneshsiva11 can you check if the example needs to be incorporated ? It looks like maybe the existing docs already cover it

vigneshsiva11 · 2026-01-17T16:03:58Z

Thanks for the review!

My intention was to improve discoverability by moving the async example into a doctest, since it closely mirrors the sync builder example and keeps the documentation closer to the API.

That said, I agree that the existing docs (e.g. new_with_metadata) may already cover this sufficiently. If you think the async example is redundant, I’m happy to remove it entirely and keep the docs minimal.

Please let me know which direction you prefer.

alamb · 2026-01-17T16:14:21Z

That said, I agree that the existing docs (e.g. new_with_metadata) may already cover this sufficiently. If you think the async example is redundant, I’m happy to remove it entirely and keep the docs minimal.

I think it covers the functionality sufficiently -- let's just remove the example and not change the existing docs

alamb

Thanks @vigneshsiva11 -- I took the liberty of refining the example a little and pushed to your branch

alamb · 2026-01-19T16:41:19Z

parquet/src/arrow/arrow_reader/mod.rs

    /// let mut builder = ParquetRecordBatchReaderBuilder::try_new(file).unwrap();
-    ///
-    /// // Inspect metadata
+    /// // The builder has access to ParquetMetaData such


I refined this example with some more comments as well as not calling with_row_groups to keep the example simpler

vigneshsiva11 · 2026-01-20T12:13:28Z

Thanks a lot, @alamb!

The refinements look great to me, and keeping the example simpler makes sense. I’m happy with this as is.

docs(parquet): move async parquet example into ArrowReaderBuilder docs

f9e2ea2

Copilot AI review requested due to automatic review settings January 14, 2026 07:40

github-actions bot added the parquet Changes to the parquet crate label Jan 14, 2026

Copilot started reviewing on behalf of vigneshsiva11 January 14, 2026 07:41 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

docs(parquet): fix ArrowReaderBuilder example consistency

9a363e9

alamb reviewed Jan 14, 2026

View reviewed changes

Vignesh S and others added 3 commits January 15, 2026 07:55

docs(parquet): add sync doctest example for ParquetRecordBatchReaderB…

d7fd9fe

…uilder

Merge branch 'main' into docs-parquet-example

763ce0a

docs(parquet): remove stale async_read_parquet example entry

a842ada

alamb approved these changes Jan 17, 2026

View reviewed changes

parquet/src/arrow/arrow_reader/mod.rs Outdated Show resolved Hide resolved

alamb reviewed Jan 17, 2026

View reviewed changes

alamb self-requested a review January 17, 2026 15:47

Vignesh S and others added 4 commits January 18, 2026 09:23

ci(parquet): remove async_read_parquet from examples

0720743

docs(parquet): fix rustdoc Vec<u8> HTML parsing

8c63f25

Apply suggestion from @alamb

9d1b2da

Refine example

c8314cf

alamb approved these changes Jan 19, 2026

View reviewed changes

-    /// let mut reader = builder.build()?;
+    /// let mut reader = builder.build()?;
+    ///
+    /// // Read all record batches from the reader
+    /// while let Some(batch) = reader.next().transpose()? {
+    ///     println!("Read {} rows", batch.num_rows());
+    /// }

docs(parquet): move async parquet example into ArrowReaderBuilder docs #9167

Are you sure you want to change the base?

docs(parquet): move async parquet example into ArrowReaderBuilder docs #9167

Conversation

vigneshsiva11 commented Jan 14, 2026

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

vigneshsiva11 commented Jan 14, 2026

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

vigneshsiva11 Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alamb Jan 17, 2026

Choose a reason for hiding this comment

Uh oh!

alamb commented Jan 17, 2026

Uh oh!

vigneshsiva11 commented Jan 17, 2026

Uh oh!

alamb commented Jan 17, 2026

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

vigneshsiva11 commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants