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

ENH: allow clients to send extra compute and odo kwargs #1342

Merged
merged 4 commits into from Dec 11, 2015

Conversation

Projects
None yet
3 participants
@llllllllll
Member

llllllllll commented Dec 4, 2015

blaze.compute accepts keyword args that can change how computations are
performed. This change forwards two extra dictionaries to the server for
use in computation and odo conversion. These are:

compute_kwargs - **unpacked into the compute call on the server
odo_kwargs - **unpacked into the odo call on the server

@cpcloud cpcloud modified the milestone: 0.9.1 Dec 7, 2015

@llllllllll llllllllll force-pushed the quantopian:server-extra-args branch 2 times, most recently from 19a49db to 5161ad9 Dec 7, 2015

@llllllllll

This comment has been minimized.

Member

llllllllll commented Dec 8, 2015

@cpcloud @kwmsmith any thoughts on this?

ENH: allow clients to send extra compute and odo kwargs
blaze.compute accepts keyword args that can change how computations are
performed. This change forwards two extra dictionaries to the server for
use in computation and odo conversion. These are:

compute_kwargs - **unpacked into the compute call on the server
odo_kwargs - **unpacked into the odo call on the server

@llllllllll llllllllll force-pushed the quantopian:server-extra-args branch from 5161ad9 to ea2f758 Dec 8, 2015

try:
return typ(x)
except TypeError:
return odo(x, typ)
return odo(x, typ, **odo_kwargs or {})

This comment has been minimized.

@kwmsmith

kwmsmith Dec 10, 2015

Member

I presume the precedence here is equivalent to **(odo_kwargs or {}), correct? What about putting explicit parens around the disjunction to make it more clear -- the spacing makes it look strange at first read.

1. ``compute_kwargs``: This is a dictionary to send to the server to use as
keyword arguments when calling ``compute`` on the server.
2. ``odo_kwargs``: This is a dictionary to send to the server to use aas

This comment has been minimized.

@kwmsmith

kwmsmith Dec 10, 2015

Member

aas -> as

@llllllllll

This comment has been minimized.

Member

llllllllll commented Dec 10, 2015

Updated to address the comments, anything else before merging?

good_query = {
'expr': to_tree(expr),
'compute_kwargs': {
'return_df': odo(DumbResource.df, list),

This comment has been minimized.

@kwmsmith

kwmsmith Dec 10, 2015

Member

There's a subtlety here -- this is the same kv pair as for the odo_kwargs above which I didn't see at first; since it ends up hitting the compute_down() dispatch on line 62, it ultimately calls dumb_to_df and everything works out.

This comment has been minimized.

@llllllllll

llllllllll Dec 10, 2015

Member

Correct. In the odo_kwargs test, expr is just a field which does not call compute_down. This changes the expr to have a sort on it causing it hit the compute_down and that is where the resource is converted into a dataframe.

@kwmsmith

This comment has been minimized.

Member

kwmsmith commented Dec 10, 2015

@llllllllll LGTM, thanks.

@llllllllll llllllllll merged commit 931bee8 into blaze:master Dec 11, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@llllllllll llllllllll deleted the quantopian:server-extra-args branch Dec 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment