Don't patch CSV#init_converters for ruby 2.5 compatibility

[rbmrclo]

Background

• csvlint library patched the method CSV#init_converters
  • https://github.com/theodi/csvlint.rb/blob/master/lib/csvlint/validate.rb#L43-L49
• It's not compatible with ruby 2.5.0
  • In earlier versions of ruby, it only accepts two arguments:
    • https://docs.ruby-lang.org/en/2.4.0/CSV.html#method-i-init_converters
  • However, in ruby 2.5.0, the required arguments are increased to three
    • https://docs.ruby-lang.org/en/2.5.0/CSV.html#method-i-init_converters
• By removing this patch, LineCSV will fallback and use the default #init_converters method of CSV for ruby 2.5.0 and above.
• Currently, when using csvlint on ruby 2.5.0, all validations are raising exceptions with :invalid_encoding message. When it's not really the cause of the issue.
  • https://github.com/theodi/csvlint.rb/blob/7e1d38934535167f3ebe12c13128650cf82d78ee/lib/csvlint/validate.rb#L169

Please review and let me know your thoughts!

cc @jpmckinney

[jpmckinney]

Seems fine to get code working in 2.5. Does 2.5 not have the same performance issue?

[rbmrclo]

@jpmckinney Thanks for the review!

Does 2.5 not have the same performance issue?

Unfortunately, I haven't gone deeper on that. Ideally, if we started to update csvlint and support 2.5, optimisation should be something that we would want to revisit again, especially on doing the same amazing improvements that you did here. ☺️

For now, the purpose of this band-aid fix is to just really keep the ball rolling on 2.5.

[Parad0X]

This bug is preventing us from upgrading some of our apps to Ruby 2.5 😞 Taking a quick look it looks like this optimization still makes sense... but this just looks terrible:

  # Optimization: Disable the CSV library's converters feature.
  # @see https://github.com/ruby/ruby/blob/v2_2_3/lib/csv.rb#L2100
  if RUBY_VERSION < '2.5'
    def init_converters(options, field_name = :converters)
      @converters = []
      @header_converters = []
      options.delete(:unconverted_fields)
      options.delete(field_name)
    end
  else
    def init_converters(converters, ivar_name, convert_method)
      instance_variable_set(ivar_name, [])
    end
  end  

[os6sense]

Currently, when using csvlint on ruby 2.5.0, all validations are raising exceptions with :invalid_encoding message. When it's not really the cause of the issue.

Spent several hours trying to work out why this was happening, thinking that there was something wrong with the csv; thanks for highlighting the issue.

[Domon]

Hi @theodi, @Floppy, @quadrophobiac,

Is there any concern stopping this PR from being merged?

The latest version of Ruby is now 2.6.1 but this bug keeps some users at 2.4.*. 😭

Update: This bug only exists on Ruby 2.5.

[gaurav-cf]

Hi @Floppy @quadrophobiac, thanks for this amazing library, but can you let us know when this PR would be merged or is there anything else required?

Thank you!

[Floppy]

Neither @quadrophobiac or I have push access to this any more as we’re not at ODI any more. But, it doesn’t look like anyone is watching over there. I will see if I can chase someone down and get this into a state where we can get some new maintainers on. I’m happy to be one of them...

[gaurav-cf]

Thank you @Floppy. And, getting this gem into an active state would be great as it provides so many cool features.

[nimashariatian]

Hi folks,
Can someone approve this merge please and make a new release?
The merging is blocked at the moment.
We are having the same issue!

[Floppy]

Good news! csvlint has been adopted by the Data Liberation Front (a few of us who wrote it originally, now ex-ODI), so I can merge this in! I agree that the optimisation can be revisited separately for ruby >= 2.5

[nimashariatian]

This is awesome, thank you @Floppy !