nevi-me commented on a change in pull request #9011:
URL: https://github.com/apache/arrow/pull/9011#discussion_r549601620
##########
File path: rust/arrow/src/ipc/writer.rs
##########
@@ -355,41 +381,83 @@ pub struct FileWriter<W: Write> {
impl<W: Write> FileWriter<W> {
/// Try create a new writer, with the schema written as part of the header
+ #[deprecated(
+ since = "3.0.0",
+ note = "This method is deprecated in favour of `let writer =
FileWriter::new(writer, schema); writer.write_header_schema().unwrap();`"
+ )]
pub fn try_new(writer: W, schema: &Schema) -> Result<Self> {
- let write_options = IpcWriteOptions::default();
- Self::try_new_with_options(writer, schema, write_options)
+ let mut me = FileWriter::new(writer, schema);
+ me.write_header_schema()?;
+ Ok(me)
}
/// Try create a new writer with IpcWriteOptions
+ #[deprecated(
+ since = "3.0.0",
+ note = "This method is deprecated in favour of `let writer =
FileWriter::new(writer, schema).with_write_options(opt);
w.write_header_schema().unwrap();`"
+ )]
pub fn try_new_with_options(
writer: W,
schema: &Schema,
write_options: IpcWriteOptions,
) -> Result<Self> {
- let data_gen = IpcDataGenerator::default();
- let mut writer = BufWriter::new(writer);
- // write magic to header
- writer.write_all(&super::ARROW_MAGIC[..])?;
- // create an 8-byte boundary after the header
- writer.write_all(&[0, 0])?;
- // write the schema, set the written bytes to the schema + header
- let encoded_message = data_gen.schema_to_bytes(schema, &write_options);
- let (meta, data) = write_message(&mut writer, encoded_message,
&write_options)?;
- Ok(Self {
- writer,
- write_options,
+ let mut me = FileWriter::new(writer, schema);
+ me.write_options = write_options;
+ me.write_header_schema()?;
+ Ok(me)
+ }
+
+ /// Creates the FileWriter.
+ pub fn new(writer: W, schema: &Schema) -> Self {
Review comment:
The major downside with a `new()` that doesn't create the header, is
that it creates a burden on end-users to remember to call the
`write_header_schema()`. I would prefer to stick with `try_new()` and
`try_new_with_options()`(happy to rename the latter), where we now amend the
latter to also take other parameters.
I don't mind us using a builder pattern, but we shouldn't change the
`try_new` in the process.
----------------------------------------------------------------
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]
