Skip to content

CLN: remove unused min_count argument in libgroupby.group_nth #37747

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

Merged
merged 4 commits into from
Nov 13, 2020

Conversation

arw2019
Copy link
Member

@arw2019 arw2019 commented Nov 10, 2020

  • closes #xxxx
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@WillAyd
Copy link
Member

WillAyd commented Nov 10, 2020

Does this have any impact to an end user?

@arw2019
Copy link
Member Author

arw2019 commented Nov 10, 2020

Does this have any impact to an end user?

Good point. I think it allows the user to pass min_count to group_nth. Will see if I can come up with a test

@@ -603,8 +603,7 @@ def _aggregate(
):
if agg_func is libgroupby.group_nth:
# different signature from the others
# TODO: should we be using min_count instead of hard-coding it?
agg_func(result, counts, values, comp_ids, rank=1, min_count=-1)
Copy link
Member

Choose a reason for hiding this comment

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

one alternative here is to remove the keyword from the libgroupby function.

Copy link
Member Author

Choose a reason for hiding this comment

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

I went with that

Actually we don't use the min_count argument in the libgroupby function - I guess that it was put there to homogenize signatures, but now they're different anyways

@arw2019 arw2019 changed the title CLN: don't hard-code min_count in call to _libs.groupby.group_nth CLN: remove unused min_count argument in _libs.groupby.group_nth Nov 11, 2020
@arw2019 arw2019 changed the title CLN: remove unused min_count argument in _libs.groupby.group_nth CLN: remove unused min_count argument in libgroupby.group_nth Nov 11, 2020
@jreback jreback added the Clean label Nov 13, 2020
@jreback jreback added this to the 1.2 milestone Nov 13, 2020
@jreback jreback merged commit c4f6588 into pandas-dev:master Nov 13, 2020
@jreback
Copy link
Contributor

jreback commented Nov 13, 2020

thanks @arw2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants