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

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

Merged
merged 4 commits into from
Dec 11, 2015

Conversation

llllllllll
Copy link
Member

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 server-extra-args branch 2 times, most recently from 19a49db to 5161ad9 Compare December 8, 2015 21:28
@llllllllll
Copy link
Member Author

@cpcloud @kwmsmith any thoughts on this?

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
try:
return typ(x)
except TypeError:
return odo(x, typ)
return odo(x, typ, **odo_kwargs or {})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@llllllllll
Copy link
Member Author

Updated to address the comments, anything else before merging?

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

@llllllllll LGTM, thanks.

@llllllllll llllllllll merged commit 931bee8 into blaze:master Dec 11, 2015
@llllllllll llllllllll deleted the server-extra-args branch December 11, 2015 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants