Skip to content
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

Implement default handler for multiple client pre_execute #1727

Closed
wants to merge 1 commit into from

Conversation

Projects
2 participants
@toryhaavik
Copy link
Contributor

commented Mar 14, 2019

No description provided.

@@ -46,6 +48,15 @@ def pre_execute_default(node, *clients, **kwargs):
return {}


# Merge the results of all client pre-execution with scope
@pre_execute.register(ops.Node, [ibis.client.Client])

This comment has been minimized.

Copy link
@cpcloud

cpcloud Mar 14, 2019

Member

Should this be:

pre_execute.register(ops.Node, ibis.client.Client, [ibis.client.Client])

Or are you intentionally matching 0 clients as well?

This comment has been minimized.

Copy link
@toryhaavik

toryhaavik Mar 14, 2019

Author Contributor

ah, no, i didn't realize it might match 0 clients. what does the function signature look like for that dispatch registration?

This comment has been minimized.

Copy link
@cpcloud

cpcloud Mar 14, 2019

Member

function signature looks like pre_execute(op, client, *clients, **kwargs)

@cpcloud cpcloud added this to the 1.0.0 milestone Mar 14, 2019

@cpcloud cpcloud added the pandas label Mar 14, 2019

@cpcloud cpcloud self-assigned this Mar 14, 2019

@cpcloud cpcloud added this to To do in Pandas via automation Mar 14, 2019

@cpcloud cpcloud added the enhancement label Mar 14, 2019

@cpcloud

This comment has been minimized.

Copy link
Member

commented Mar 15, 2019

LGTM. Will merge later today.

@cpcloud cpcloud closed this in d68f468 Mar 17, 2019

Pandas automation moved this from To do to Done Mar 17, 2019

@cpcloud

This comment has been minimized.

Copy link
Member

commented Mar 17, 2019

Thanks @toryhaavik!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.