-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
CLN: simplify _shallow_copy #29474
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
CLN: simplify _shallow_copy #29474
Conversation
Is |
huh, looks like it isnt. any strong opinion on where it would be moved? |
Maybe merge into WindowMixin. If it’s not actually used for GroupBy I don’t even think we need to keep the GroupByMixin name as that would be rather misleading
…Sent from my iPhone
On Nov 7, 2019, at 5:14 PM, jbrockmendel ***@***.***> wrote:
huh, looks like it isnt. any strong opinion on where it would be moved?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
let me look I made these for consistency between group by and window ops |
For now at least I'm leaning towards keeping it where it is. it would need to go into some shared-by-resample/window location, which describes a lot of groupby code anyway. |
@@ -26,7 +26,23 @@ | |||
""" | |||
|
|||
|
|||
class _GroupByMixin(GroupByMixin): | |||
def _dispatch(name: str, *args, **kwargs): |
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.
eventually add a full doc-string if we end up using this
thxs |
The class hierarchy of in/around groupby is really tough, this simplifies it a little bit