Remove delimiter from csv Writer

[gsserge]

Which issue does this PR close?

Closes #1328.

I think that I wrote the issue above in a bit of a rush, the part about changes to Writer::new(). The doc comment for Writer::new() clearly states that a new CsvWriter is initialized with default options, with , being the default delimiter.

A custom delimiter can be configured using already existing WriterBuilder::with_delimiter. So, in my view, the scope of the issue is to simply remove unused code without any changes to the API.

Rationale for this change

There is no use for the delimiter field in csv Writer.

What changes are included in this PR?

The unused delimiter field is removed.

Are there any user-facing changes?

No.

[codecov-commenter]

Codecov Report

Merging #1342 (333672c) into master (ecba7dc) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1342   +/-   ##
=======================================
  Coverage   83.04%   83.04%           
=======================================
  Files         181      181           
  Lines       52937    52937           
=======================================
  Hits        43960    43960           
  Misses       8977     8977           

Impacted Files	Coverage Δ
arrow/src/csv/writer.rs	72.13% <ø> (ø)
arrow/src/datatypes/datatype.rs	66.40% <0.00%> (-0.40%)	⬇️
parquet_derive/src/parquet_field.rs	66.21% <0.00%> (-0.23%)	⬇️
arrow/src/array/transform/mod.rs	84.65% <0.00%> (+0.13%)	⬆️
arrow/src/datatypes/field.rs	54.10% <0.00%> (+0.30%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ecba7dc...333672c. Read the comment docs.

[Dandandan]

I think the delimiter is not used in the builder (when converting to Writer), so I think the original issue description is correct?

[gsserge]

    /// Create a new `Writer`
    pub fn build<W: Write>(self, writer: W) -> Writer<W> {
        let delimiter = self.delimiter.unwrap_or(b','); // looks like it's used
        let mut builder = csv_crate::WriterBuilder::new();
        let writer = builder.delimiter(delimiter).from_writer(writer);
        ...

[Dandandan]

    /// Create a new `Writer`
    pub fn build<W: Write>(self, writer: W) -> Writer<W> {
        let delimiter = self.delimiter.unwrap_or(b','); // looks like it's used
        let mut builder = csv_crate::WriterBuilder::new();
        let writer = builder.delimiter(delimiter).from_writer(writer);
        ...

Yes it's used in the builder, but looks to me it's not passed to the Writer eventually (and so not used when writing the CSV).

[gsserge]

Hm, but what about this line

let writer = builder.delimiter(delimiter).from_writer(writer);

If I understand correctly, builder here is from the csv crate, and it gets the delimiter configuration.

[Dandandan]

Hm, but what about this line

let writer = builder.delimiter(delimiter).from_writer(writer);

If I understand correctly, builder here is from the csv crate, and it gets the delimiter configuration.

Ah yeah, you are right 👍 missed that this was the Writer from the CSV crate.

Maybe we could add a test for writing a csv with another separator?

[gsserge]

There's already a test for custom options, including the delimiter: https://github.com/apache/arrow-rs/blob/master/arrow/src/csv/writer.rs#L637

[Dandandan]

There's already a test for custom options, including the delimiter: https://github.com/apache/arrow-rs/blob/master/arrow/src/csv/writer.rs#L637

Perfect, thanks!