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

context manager for scheduled functions #828

Merged
merged 3 commits into from Nov 11, 2015

Conversation

Projects
None yet
2 participants
@llllllllll
Member

llllllllll commented Nov 10, 2015

No description provided.

@llllllllll llllllllll force-pushed the event-manager-context branch from ab89774 to b04e1a9 Nov 10, 2015

post = post if post is not None else _nop

@contextmanager
def _callback_manager_context(*args, **kwargs):

This comment has been minimized.

@richafrank

richafrank Nov 11, 2015

Member

Instead of closing over pre and post, what do you think about factoring out a class? I think it'd be a bit more readable, and we'd get a performance increase over @contextmanager.

This comment has been minimized.

@llllllllll

llllllllll Nov 11, 2015

Member

sounds good.

@@ -169,14 +169,35 @@ def _build_time(time, kwargs):
return datetime.time(**kwargs)


class EventManager(object):
@object.__new__
class _nop_context(object):

This comment has been minimized.

@richafrank

richafrank Nov 11, 2015

Member

How does this compare to ExitStack?

This comment has been minimized.

@llllllllll

llllllllll Nov 11, 2015

Member

I wasn't sure if we used contextlib32 in zipline. This is mainly for clarity though, we never want the features of exitstack.

This comment has been minimized.

@llllllllll

llllllllll Nov 11, 2015

Member

okay, so we are using exit stack, I am still not sure if that is more clear here.

This comment has been minimized.

@richafrank

richafrank Nov 11, 2015

Member

That's fair. nop is certainly clearer.

@llllllllll llllllllll force-pushed the event-manager-context branch from f8790d4 to 88a53fb Nov 11, 2015

@llllllllll

This comment has been minimized.

Member

llllllllll commented Nov 11, 2015

Merging on pass

llllllllll added some commits Nov 10, 2015

ENH: update context_tricks
Moves _nop_context to context_tricks under the name nop_context.
Uses an explicit object for the actual context manager in callback
manager.

@llllllllll llllllllll merged commit 88a53fb into master Nov 11, 2015

2 checks passed

Scrutinizer 17 updated code elements
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@llllllllll llllllllll deleted the event-manager-context branch Nov 11, 2015

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