Copilot commented on code in PR #9167:
URL: https://github.com/apache/arrow-rs/pull/9167#discussion_r2689332047
##########
parquet/src/arrow/arrow_reader/mod.rs:
##########
@@ -1002,6 +1002,20 @@ impl<T: ChunkReader + 'static>
ParquetRecordBatchReaderBuilder<T> {
/// // Read data
/// let _batch = reader.next().unwrap().unwrap();
/// ```
+
+
+ /// # 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()?;
Review Comment:
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.
```suggestion
/// 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());
/// }
```
##########
parquet/src/arrow/arrow_reader/mod.rs:
##########
@@ -1002,6 +1002,20 @@ impl<T: ChunkReader + 'static>
ParquetRecordBatchReaderBuilder<T> {
/// // Read data
/// let _batch = reader.next().unwrap().unwrap();
/// ```
+
+
+ /// # 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(())
+ /// # }
+ /// ```
+
Review Comment:
Extra blank lines before and after the example section. Standard Rust
documentation style typically uses a single blank line between doc comment
sections.
##########
parquet/src/arrow/arrow_reader/mod.rs:
##########
@@ -1002,6 +1002,20 @@ impl<T: ChunkReader + 'static>
ParquetRecordBatchReaderBuilder<T> {
/// // Read data
/// let _batch = reader.next().unwrap().unwrap();
/// ```
+
+
+ /// # 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(())
+ /// # }
+ /// ```
Review Comment:
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.
##########
parquet/src/arrow/arrow_reader/mod.rs:
##########
@@ -1002,6 +1002,20 @@ impl<T: ChunkReader + 'static>
ParquetRecordBatchReaderBuilder<T> {
/// // Read data
/// let _batch = reader.next().unwrap().unwrap();
/// ```
+
+
+ /// # Example
+ /// ```rust
Review Comment:
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.
```suggestion
/// ```rust,no_run
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
