-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
Conversation
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
pls rebase (though i see a similar PR adding some of this machinery), so either ok (suspect they overlap a bit) |
rebased + green |
thanks. not super happy with adding state, but as you are working towards more robust ways this is fine. |
making it incrementally easier to reason about what
self.grouper
is