Skip to content

Add apply function like IPython's apply#357

Merged
kwmsmith merged 6 commits into
enthought:masterfrom
cowlicks:da-apply
May 6, 2014
Merged

Add apply function like IPython's apply#357
kwmsmith merged 6 commits into
enthought:masterfrom
cowlicks:da-apply

Conversation

@cowlicks
Copy link
Copy Markdown
Contributor

@cowlicks cowlicks commented May 2, 2014

No description provided.

Comment thread distarray/context.py Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

apply should take a flag argument that indicates whether it should return the result of func on the engines directly, or if it should store the result on the engines like here. Sometimes we want the result returned to the client, other times we want the result stored on the engines and the name returned.

@cowlicks
Copy link
Copy Markdown
Contributor Author

cowlicks commented May 5, 2014

@kwmsmith I added the flag with tests.

@bgrant bgrant added this to the 0.3 milestone May 5, 2014
@kwmsmith
Copy link
Copy Markdown
Contributor

kwmsmith commented May 5, 2014

@cowlicks Better, but there are usecases where we don't want the result stored on the engines at all, just returned directly to the end user. I think a more straightforward option that would cover this usecase would be something like:

def func_wrapper(func, result_name, args, kwargs):
    # ... args and kwargs processing code as before ...
    result = func(*args, **kwargs)
    if result_name:
        setattr(main, result_name, result)
        result = result_name
    return result

And then the apply() function would have an argument result_name=None, and no return_name=True argument. This way we only store objects on the engines if explicitly asked to do so.

@cowlicks
Copy link
Copy Markdown
Contributor Author

cowlicks commented May 5, 2014

I think the default behavior should be returning the name since it is less costly to send than the potentially large set of results.

@cowlicks
Copy link
Copy Markdown
Contributor Author

cowlicks commented May 5, 2014

Now when result_name is passed in, A variable of that name is created on the engines, and result_name is returned to act as a proxy to the result on the engines.

if result_name is None then a list of the result on each engine is returned.

Comment thread distarray/dist/context.py Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor question: why use *args and **kwargs rather than just passing in a list for args and a dict for kwargs? It would clean up some of the logic for calling func_wrapper below inside apply.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I originally did this because _really_apply expands the arguments. However I can work around this.

@cowlicks
Copy link
Copy Markdown
Contributor Author

cowlicks commented May 6, 2014

@kwmsmith noted

@kwmsmith kwmsmith merged commit 4336a78 into enthought:master May 6, 2014
@kwmsmith
Copy link
Copy Markdown
Contributor

kwmsmith commented May 6, 2014

@cowlicks looks great, merging.

@cowlicks cowlicks deleted the da-apply branch May 6, 2014 22:11
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.

3 participants