alamb commented on code in PR #6132:
URL: https://github.com/apache/arrow-rs/pull/6132#discussion_r1693934161
##########
arrow-ipc/src/reader.rs:
##########
@@ -1043,6 +1043,15 @@ impl<R: Read + Seek> fmt::Debug for FileReader<R> {
}
}
+impl<R: Read + Seek> FileReader<BufReader<R>> {
+ /// Try to create a new file reader with the reader wrapped in a BufReader.
+ ///
+ /// See [`FileReader::try_new`] for an unbuffered version.
Review Comment:
Could you also please add a note to `FileReader::try_new` that it does not
do any internal buffering, and if users want buffering they should see
`FileReader::try_new_buffered`?
##########
arrow-ipc/src/writer.rs:
##########
@@ -844,6 +844,15 @@ pub struct FileWriter<W: Write> {
data_gen: IpcDataGenerator,
}
+impl<W: Write> FileWriter<BufWriter<W>> {
+ /// Try to create a new file writer with the writer wrapped in a BufWriter.
+ ///
+ /// See [`FileWriter::try_new`] for an unbuffered version.
+ pub fn try_new_buffered(writer: W, schema: &Schema) -> Result<Self,
ArrowError> {
+ Self::try_new(BufWriter::new(writer), schema)
+ }
+}
+
impl<W: Write> FileWriter<W> {
/// Try to create a new writer, with the schema written as part of the
header
Review Comment:
```suggestion
/// Try to create a new writer, with the schema written as part of the
header
///
/// Note the created writer is not buffered. See
[`FileWriter::try_new_buffered`] for details.
```
##########
arrow-ipc/src/writer.rs:
##########
@@ -990,16 +998,20 @@ impl<W: Write> FileWriter<W> {
Ok(())
}
- /// Unwraps the BufWriter housed in FileWriter.writer, returning the
underlying
- /// writer
+ /// Unwraps the the underlying writer.
+ ///
+ /// The writer is flushed and the FileWriter is finished before returning.
///
- /// The buffer is flushed and the FileWriter is finished before returning
the
- /// writer.
+ /// # Errors
+ ///
+ /// An ['Err'](Result::Err) may be returned if an error occurs while
finishing the StreamWriter
Review Comment:
👍
##########
arrow-ipc/src/writer.rs:
##########
@@ -853,12 +862,11 @@ impl<W: Write> FileWriter<W> {
/// Try to create a new writer with IpcWriteOptions
Review Comment:
```suggestion
/// Try to create a new writer with IpcWriteOptions
///
/// Note the created writer is not buffered. See
[`FileWriter::try_new_buffered`] for details.
```
##########
arrow-ipc/src/reader.rs:
##########
@@ -1163,21 +1172,24 @@ impl<R: Read> fmt::Debug for StreamReader<R> {
}
impl<R: Read> StreamReader<BufReader<R>> {
- /// Try to create a new stream reader with the reader wrapped in a
BufReader
+ /// Try to create a new stream reader with the reader wrapped in a
BufReader.
///
- /// The first message in the stream is the schema, the reader will fail if
it does not
- /// encounter a schema.
- /// To check if the reader is done, use `is_finished(self)`
- pub fn try_new(reader: R, projection: Option<Vec<usize>>) -> Result<Self,
ArrowError> {
- Self::try_new_unbuffered(BufReader::new(reader), projection)
+ /// See [`StreamReader::try_new`] for an unbuffered version.
+ pub fn try_new_buffered(reader: R, projection: Option<Vec<usize>>) ->
Result<Self, ArrowError> {
+ Self::try_new(BufReader::new(reader), projection)
}
}
impl<R: Read> StreamReader<R> {
- /// Try to create a new stream reader but do not wrap the reader in a
BufReader.
+ /// Try to create a new stream reader.
+ ///
+ /// The first message in the stream is the schema, the reader will fail if
it does not
+ /// encounter a schema.
+ /// To check if the reader is done, use
[`is_finished(self)`](StreamReader::is_finished).
///
- /// Unless you need the StreamReader to be unbuffered you likely want to
use `StreamReader::try_new` instead.
- pub fn try_new_unbuffered(
Review Comment:
To ease migration, what do you think about adding a `try_new_unbuffered`
function (that calls `try_new`) marked as `deprecated` ?
--
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]
