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

chore(python): Rename pivot aggregate_fn to aggregate_function #7059

Merged
merged 1 commit into from Feb 21, 2023

Conversation

stinodego
Copy link
Member

Follow-up of #7032

I am not 100% sure about this one.

aggregate_function feels too long for an argument that should usually be called as a keyword argument.

I think agg is nice - it's short and is the same as groupby.agg, which takes similar input.

But I am open to better ideas.

@github-actions github-actions bot added chore Maintenance work that does not impact the user python Related to Python Polars labels Feb 20, 2023
@ritchie46
Copy link
Member

We go from intially more explicit to less now. I think the function has merit in explaining the keyword. So either aggrefate_fn is ok, or proposed aggregate_function or if deemed too long agg_fn.

@stinodego stinodego changed the title chore(python): Rename pivot aggregate_fn to agg chore(python): Rename pivot aggregate_fn to aggregate_function Feb 21, 2023
@stinodego
Copy link
Member Author

stinodego commented Feb 21, 2023

Right, let's just be consistent and make it aggregate_function.

I thought the implicit link with the groupby.agg method was sort-of nice, but I think this is better in line with our naming conventions.

@stinodego stinodego merged commit e703ac2 into pola-rs:master Feb 21, 2023
josemasar pushed a commit to josemasar/polars that referenced this pull request Feb 21, 2023
@stinodego stinodego deleted the pivot-agg-fn branch February 22, 2023 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Maintenance work that does not impact the user python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants