Skip to content

REF: use public indexers in groupby.ops #31814

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 5 commits into from
Feb 9, 2020

Conversation

jbrockmendel
Copy link
Member

No description provided.

@jreback jreback added the Groupby label Feb 9, 2020
@@ -780,7 +780,11 @@ def get_iterator(self, data: FrameOrSeries, axis: int = 0):
Generator yielding sequence of (name, subsetted object)
for each group
"""
slicer = lambda start, edge: data._slice(slice(start, edge), axis=axis)
if axis == 0:
Copy link
Contributor

Choose a reason for hiding this comment

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

a more natural idiom (used in many places is)

indexer = [None] * 2
indexer[axis] = slice(start, edge)
slicer = data.iloc[indexer]

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 guess, but thats not really conducive to being a lambda

Copy link
Contributor

Choose a reason for hiding this comment

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

why does this need to be a lamba?

Copy link
Member Author

Choose a reason for hiding this comment

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

doesnt have to be, thats just the pattern we have now. i find iloc[:, start:end] clearer than the 3-line version

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@jreback jreback added the Refactor Internal refactoring of code label Feb 9, 2020
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

Lgtm outside of Jeff’s comment(s)

@jreback jreback added this to the 1.1 milestone Feb 9, 2020
@jreback jreback merged commit 8cfcf65 into pandas-dev:master Feb 9, 2020
@jbrockmendel jbrockmendel deleted the _get_values branch February 9, 2020 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Groupby Refactor Internal refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants