polars icon indicating copy to clipboard operation
polars copied to clipboard

feat(rust): Improves documentation for GCP and adds setters to ScanArgsParquet

Open andyquinterom opened this issue 1 year ago • 3 comments

This is a very small PR that makes some small quality of life improvements.

I added some setters for ScanArgsParquet to make it easier to built these options.

I added some documentation for CloudOptions with_gcp to make it more clear on how to use.

andyquinterom avatar Apr 19 '24 05:04 andyquinterom

CodSpeed Performance Report

Merging #15759 will not alter performance

Comparing andyquinterom:improve-cloud-dx (a41552b) with main (d11da5e)

Summary

✅ 21 untouched benchmarks

codspeed-hq[bot] avatar Apr 19 '24 06:04 codspeed-hq[bot]

Codecov Report

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

Project coverage is 81.35%. Comparing base (96e1f01) to head (a41552b). Report is 35 commits behind head on main.

Files Patch % Lines
crates/polars-lazy/src/scan/parquet.rs 0.00% 36 Missing :warning:
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #15759    +/-   ##
========================================
  Coverage   81.35%   81.35%            
========================================
  Files        1379     1379            
  Lines      176603   176879   +276     
  Branches     2544     2543     -1     
========================================
+ Hits       143677   143904   +227     
- Misses      32443    32493    +50     
+ Partials      483      482     -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 19 '24 18:04 codecov[bot]

I don't really see the point of setter methods on a struct with public attributes?

Hey! Thanks for taking the time to review this PR. I would say the main reason is ergonomics. For nested functions it's easier than declaring a new mutable variable and modifying it or using the ..Default::default() syntax.

I see other important structs like the CsvReader have setters which confused me for a while thinking that ScanArgsParquet would also.

Perhaps it's worth thinking of a unified setter/getter language for the different crates?

andyquinterom avatar Apr 23 '24 14:04 andyquinterom