-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CLN: move small bits outside of try/excepts #28962
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
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.
lgtm, thanks @jbrockmendel
@@ -292,12 +292,10 @@ def _try_aggregate_string_function(self, arg, *args, **kwargs): | |||
|
|||
f = getattr(np, arg, None) | |||
if f is not None: | |||
try: | |||
if hasattr(self, "__array__"): |
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.
Do we need to account for __array_ufunc__
or __array_function__
here as well? @TomAugspurger might want to take a look
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.
IIUC, this change is OK since we're checking if self
has __array__
, which our Index / Series objects have (in addition to array_ufunc)
@@ -598,14 +598,7 @@ def pipe(self, func, *args, **kwargs): | |||
plot = property(GroupByPlot) | |||
|
|||
def _make_wrapper(self, name): | |||
if name not in self._apply_whitelist: |
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.
Nice cleanup. At a glance this looks like a pretty interesting factory...
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.
Yah, this used to be called from within __gettattr__
(and exec
was involved) Recently changed to be called from properties generated directly from apply_whitelist, so this check is no longer necessary
thanks |
No description provided.