Skip to content

CLN: Some groupby internals #31915

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 3 commits into from
Feb 17, 2020
Merged

Conversation

mroeschke
Copy link
Member

  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  1. Avoids calling _get_sorted_data() twice
  2. Eliminates *args, **kwargs from an internal function

@WillAyd
Copy link
Member

WillAyd commented Feb 12, 2020

Looks like a segfault on travis - restarted to see

@@ -941,11 +939,13 @@ def _chop(self, sdata: DataFrame, slice_obj: slice) -> DataFrame:
return sdata.iloc[:, slice_obj]


def get_splitter(data: FrameOrSeries, *args, **kwargs) -> DataSplitter:
def get_splitter(
data: FrameOrSeries, labels, ngroups: int, axis: int = 0
Copy link
Member

Choose a reason for hiding this comment

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

is labels an Index?

@@ -927,11 +927,9 @@ def _chop(self, sdata: Series, slice_obj: slice) -> Series:


class FrameSplitter(DataSplitter):
def fast_apply(self, f, names):
def fast_apply(self, f, sdata, names):
Copy link
Member

Choose a reason for hiding this comment

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

is sdata a DataFrame?

@jbrockmendel
Copy link
Member

potential annotations; LGTM pending green

@WillAyd
Copy link
Member

WillAyd commented Feb 12, 2020

Can you merge master? Slightly concerned about reproducibility of previous segfault

@mroeschke
Copy link
Member Author

Thanks for the review. Merged master and all green

@mroeschke mroeschke merged commit 06eb8db into pandas-dev:master Feb 17, 2020
@mroeschke mroeschke deleted the groupby_cleanups branch February 17, 2020 00:23
roberthdevries pushed a commit to roberthdevries/pandas that referenced this pull request Mar 2, 2020
* CLN: Some groupby internals

* Additional annotation
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.

3 participants