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

docs(python): Update leftover references of by parameter to group_by in DataFrame/LazyFrame.upsample/group_by_dynamic/rolling #15527

Merged
merged 1 commit into from
Apr 7, 2024

Conversation

henryharbeck
Copy link
Contributor

@henryharbeck henryharbeck commented Apr 7, 2024

#14840 renamed the by parameter to group_by in

  • DataFrame.upsample/group_by_dynamic/rolling
  • LazyFrame.group_by_dynamic/rolling

This PR cleans up remaining references in the docstrings to the old by paramter.

@github-actions github-actions bot added documentation Improvements or additions to documentation python Related to Python Polars labels Apr 7, 2024
@@ -3242,7 +3242,7 @@ def rolling(
"""
Create rolling groups based on a temporal or integer column.

Different from a `dynamic_group_by` the windows are now determined by the
Different from a `group_by_dynamic` the windows are now determined by the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Drive by typo fix. Now aligns with DataFrame docstring.

of which will be sorted by `index_column` (but note that if `by` columns are
passed, it will only be sorted within each `by` group).
of which will be sorted by `index_column` (but note that if `group_by`
columns are passed, it will only be sorted within each group).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that the by in "...within each by group)." isn't needed anymore.
The group_by parameter name makes it very obvious what the group is.

Copy link
Collaborator

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Nice one @henryharbeck , good one!

Copy link

codecov bot commented Apr 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.15%. Comparing base (1d4880f) to head (d5f5472).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #15527   +/-   ##
=======================================
  Coverage   81.14%   81.15%           
=======================================
  Files        1362     1362           
  Lines      175007   175007           
  Branches     2533     2533           
=======================================
+ Hits       142014   142031   +17     
+ Misses      32509    32491   -18     
- Partials      484      485    +1     

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

@alexander-beedie alexander-beedie merged commit 63ad8af into pola-rs:main Apr 7, 2024
14 checks passed
@henryharbeck henryharbeck deleted the by_rename branch April 7, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants