Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename functions to read CSV #305

Merged
merged 9 commits into from
Jul 4, 2023
Merged

Rename functions to read CSV #305

merged 9 commits into from
Jul 4, 2023

Conversation

etiennebacher
Copy link
Collaborator

@etiennebacher etiennebacher commented Jul 4, 2023

Mentioned in #267 (comment)

This PR:

  • lazy_csv_reader -> scan_csv
  • csv_reader -> read_csv

For now, they only can be accessed with pl$ because I removed read_csv_, but if we still want to have this kind of high-level helper, we should rename it (maybe read_csv_pl()?)

Feel free to push directly here

@eitsupi
Copy link
Collaborator

eitsupi commented Jul 4, 2023

Thank you for working on this!

I thought we should do this too, but when I looked at the py-polars implementation I realized that Python's polars.scan_csv is completely different from the current R's lazy_csv_reader.

That is, Python's scan_csv seems to map directly to Rust's scan_csv.
https://github.com/pola-rs/polars/blob/d40e403c81620f659244af20ce36470af8afe419/polars/polars-lazy/polars-plan/src/logical_plan/builder.rs#L206

@sorhawell
Copy link
Collaborator

sorhawell commented Jul 4, 2023

actually py-polars uses also LazyCsvReader via here and via here

I have no idea what the difference between scan_csv and LazyCsvReader in rust-polars are. I was not aware of scan_csv in rust-polars.

Copy link
Collaborator

@eitsupi eitsupi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the updates! I think it is a good change for consistency.

Perhaps we can merge this since it seems to have passed the test?

@eitsupi
Copy link
Collaborator

eitsupi commented Jul 4, 2023

actually py-polars uses also LazyCsvReader via here and via here

Oh, sorry. I must have misread the code.......

@sorhawell sorhawell merged commit 08fc240 into main Jul 4, 2023
10 checks passed
@sorhawell sorhawell deleted the rename-csv-funs branch July 4, 2023 15:15
vincentarelbundock pushed a commit to vincentarelbundock/r-polars that referenced this pull request Jul 4, 2023
Co-authored-by: sorhawell <sorhawell@gmail.com>
Co-authored-by: eitsupi <ts1s1andn@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants