houqp commented on a change in pull request #7666:
URL: https://github.com/apache/arrow/pull/7666#discussion_r451278084
##########
File path: rust/arrow/src/record_batch.rs
##########
@@ -216,15 +216,28 @@ impl Into<StructArray> for RecordBatch {
}
}
-/// Definition of record batch reader.
+/// Trait for types that can read `RecordBatch`'s in a single-threaded context.
pub trait RecordBatchReader {
- /// Returns schemas of this record batch reader.
- /// Implementation of this trait should guarantee that all record batches
returned
- /// by this reader should have same schema as returned from this method.
+ /// Returns the schema of this `RecordBatchReader`.
+ ///
+ /// Implementation of this trait should guarantee that all `RecordBatch`'s
returned by this
+ /// reader should have the same schema as returned from this method.
fn schema(&mut self) -> SchemaRef;
- /// Returns next record batch.
- fn next_batch(&mut self) -> Result<Option<RecordBatch>>;
+ /// Reads the next `RecordBatch`.
+ fn next(&mut self) -> Result<Option<RecordBatch>>;
Review comment:
If we are to introduce breaking change in this patch set, I think it's
better to use `next_batch` or other method name instead of `next` everywhere so
it's easier for users to implement Iterator trait for the same type if they
want.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
