ruby / csv

CSV Reading and Writing
https://ruby.github.io/csv/
BSD 2-Clause "Simplified" License
178 stars 113 forks source link

CSV `header_row?` not working as expected for empty CSV writen with `CSV::Writer` #214

Closed prashantvithani closed 3 years ago

prashantvithani commented 3 years ago
2.7.3 :001 > require 'csv'
 => true 
2.7.3 :002 > csv_writer = CSV.open('dummy.csv', 'w', headers: true)
 => #<CSV io_type:File io_path:"dummy.csv" encoding:UTF-8 lineno:0 col_sep:"," row_sep:"\n" quote_char:"\"" headers:true> 
2.7.3 :003 > csv_writer.header_row? # This is correct since CSV is empty
 => true 
2.7.3 :004 > csv_writer << ['col1', 'col2', 'col3']
 => #<CSV io_type:File io_path:"dummy.csv" encoding:UTF-8 lineno:1 col_sep:"," row_sep:"\n" quote_char:"\"" headers:["col1", "col2", "col3"]> 
2.7.3 :005 > csv_writer.header_row? # This should be false
 => true 

Definition of header_row? returns parser.header_row?. Parser and Writer are initialized separately and both maintain their own copy of @headers. So, when a writer writes headers to the file and sets @headers, it's not updated in Parser and parser.header_row? always returns true.

It worked fine in 1.0.2

prashantvithani commented 3 years ago

A patch I can think of:

# csv/writer.rb
class CSV
  ...
  class Writer
    ...
    def header_row?
      @use_header && @headers.nil?
    end
    ...
  end
end

# csv.rb
class CSV
  ...
  def header_row?
    if @writer
      writer.header_row?
    else
      parser.header_row?
    end
  end
  ...
end

It's not the best but works for a temporary solution.

cc: @kou

kou commented 3 years ago

header_row? is for reader not writer. Could you show why you want to use header_row? for writer?

prashantvithani commented 3 years ago

header_row? is for reader not writer.

Ah, that makes sense. I was using header_row? on the writer to check whether to write headers or just append the row as is. The row here is a simple ruby hash, the keys (headers) are known while reading the hash.

However, I see the problem in the way I'm using it and checking header_row? for every row which is being written. What do you think about it?

kou commented 3 years ago

How about using csv_writer.lineno.zero? instead of csv_writer.header_row??

prashantvithani commented 3 years ago

That works for my use-case, thanks for the suggestion!

kou commented 3 years ago

OK. I close this.