feat(rust!): Rename to CsvParserOptions to CsvReaderOptions, use in CsvReader

[stinodego]

Changes

• Rename CsvParserOptions to CsvReaderOptions
• Allow passing a CsvReaderOptions to CsvReader.

There are many more places internally where we can use CsvReaderOptions - this will be implemented in followup PRs

[stinodego]

I removed the arbitrary 5 character limit here. Not sure why it was there in the first place 🤔 the original PR/issue do not mention anything about the limit: #12519

[mcrumiller]

The API mentions the 5 character limit here, so that should probably updated too.

[stinodego]

Good point, updated. Thanks!

[codspeed-hq]

CodSpeed Performance Report

Merging #15919 will improve performances by 22.17%

_{Comparing csvreader-options (79c3cc8) with main (a9c8b59)}

Summary

⚡ 1 improvements
✅ 12 untouched benchmarks

Benchmarks breakdown

	Benchmark	main	csvreader-options	Change
⚡	test_filter2	2.8 ms	2.3 ms	+22.17%

[codecov]

Codecov Report

Attention: Patch coverage is 94.92754% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 81.25%. Comparing base (dee176c) to head (79c3cc8).

Files	Patch %	Lines
crates/polars-io/src/csv/read/options.rs	54.54%	5 Missing ⚠️
crates/polars-io/src/csv/read/reader.rs	98.38%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15919      +/-   ##
==========================================
+ Coverage   81.24%   81.25%   +0.01%     
==========================================
  Files        1382     1382              
  Lines      176628   176615      -13     
  Branches     3032     3032              
==========================================
+ Hits       143494   143505      +11     
+ Misses      32649    32627      -22     
+ Partials      485      483       -2     

Flag	Coverage Δ
python	74.69% <94.92%> (+0.01%)	⬆️
rust	78.37% <94.92%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ritchie46]

To keep going back and forth, I am more fan of CsvParserOptions as the reader accepts options that are not fit for the parser, e.g. globbing, hive partitioning. They are passed to the reader, but not to the actual parser.

[stinodego]

We can revert before the next Rust release if you want.

But I don't think CsvParserOptions makes sense if we consider all file formats. Parquet will have an options struct but not really a 'parser' since it's not flat text. So ParquetParserOptions would be wrong.

I think all Parser structs will accept multiple options structs: CloudOptions, PartitioningOptions, perhaps others. And options specific to that file format. Since the options differ between reading and writing, it makes sense to have ParquetReaderOptions and ParquetWriterOptions, same for Csv/Ipc/etc.

I don't think it's confusing for those to exist next to a CloudOptions struct and possibly others. Maybe CsvReadOptions/CsvWriteOptions would be less confusing?

[nameexhaustion]

@stinodego , @ritchie46 , I'm currently doing some work to improve the way we read lists of CSV files. As part of that I need to do some refactoring around the options structs, mainly so that they're cheaper to clone. Can I take it from here?

[stinodego]

@nameexhaustion go for it!