Skip to content

REF: do less in Grouping.__init__ #41375

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
May 17, 2021
Merged

Conversation

jbrockmendel
Copy link
Member

making it incrementally easier to reason about what self.grouper is

elif isinstance(self.grouper, (list, tuple)):
self.grouper = com.asarray_tuplesafe(self.grouper)

# a passed Categorical
elif is_categorical_dtype(self.grouper):
self._passed_categorical = True
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't you just make this a property of the grouper itself?

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 think what you're suggesting is something like

@property
def _passed_categorical(self):
    return is_categorical_dtype(self.grouper)

that is the longer-term goal, but ATM we treat directly-passed Categoricals differently from Categoricals derived within __init__, so that property wouldn't be quite the same as this variable. I expect that the different treatment is non-intentional, but need to track that down to be sure.

@jreback
Copy link
Contributor

jreback commented May 12, 2021

pls rebase (though i see a similar PR adding some of this machinery), so either ok (suspect they overlap a bit)

@jbrockmendel
Copy link
Member Author

rebased + green

@jbrockmendel
Copy link
Member Author

@jreback gentle ping; this has become a blocker for #41373

@jreback jreback added this to the 1.3 milestone May 17, 2021
@jreback jreback merged commit fea99c3 into pandas-dev:master May 17, 2021
@jreback
Copy link
Contributor

jreback commented May 17, 2021

thanks. not super happy with adding state, but as you are working towards more robust ways this is fine.

@jbrockmendel jbrockmendel deleted the ref-grouper-2 branch May 17, 2021 14:41
TLouf pushed a commit to TLouf/pandas that referenced this pull request Jun 1, 2021
JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants