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

depr(python,rust!): Rename with_row_count to with_row_index #13494

Merged
merged 7 commits into from
Jan 8, 2024
Merged

Conversation

stinodego
Copy link
Member

@stinodego stinodego commented Jan 6, 2024

Ref #12420

The "row count" of a dataframe is its total number of rows. That's not what this method calculates. What it really does is add row numbers, as was already indicated by its default name "row_nr".

It also matches the SQL window function "ROW NUMBER" (as pointed out by Alex):
https://www.postgresql.org/docs/current/functions-window.html

This PR renames with_row_count to with_row_number, and the default column name from "row_nr" to "row_number".

Apparently there are a LOT of references to "row count" - e.g. read_csv parameters and others. I'm doing those separately as this PR was already unexpectedly large.

EDIT: We decided to go with with_row_index instead. Partly to avoid confusion with the SQL function which starts at 1 (we start at 0).

@github-actions github-actions bot added breaking rust Change that breaks backwards compatibility for the Rust crate deprecation Add a deprecation warning to outdated functionality python Related to Python Polars rust Related to Rust Polars labels Jan 6, 2024
@stinodego stinodego marked this pull request as ready for review January 6, 2024 21:47
@ritchie46
Copy link
Member

Seems fine by my. If code only renamed the methods it can be merged.

@stinodego stinodego marked this pull request as draft January 7, 2024 15:16
@stinodego stinodego changed the title depr(python,rust!): Rename with_row_count to with_row_number depr(python,rust!): Rename with_row_count to with_row_index Jan 8, 2024
@stinodego
Copy link
Member Author

Based on the discussion in the linked issue and on Discord, let's go with with_row_index instead. The default column name will become "index".

@stinodego stinodego marked this pull request as ready for review January 8, 2024 05:44
@stinodego stinodego merged commit 3077ba5 into main Jan 8, 2024
29 checks passed
@stinodego stinodego deleted the row-number branch January 8, 2024 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation breaking rust Change that breaks backwards compatibility for the Rust crate deprecation Add a deprecation warning to outdated functionality python Related to Python Polars rust Related to Rust Polars
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants