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

WebAgg: raise WebAggApplication.started flag before blocking #2865

Merged
merged 1 commit into from Mar 7, 2014

Conversation

vmarkovtsev
Copy link
Contributor

Since Tornado's IOLoop start() blocks until the loop is closed,
cls.started used to be set to True after an application actually
stops working.
Revert running() check since it no longer exists in newer versions
of Tornado and will never return back.

@pelson
Copy link
Member

pelson commented Mar 5, 2014

👍 - seems reasonable to me.

@tacaswell
Copy link
Member

Does the started flag ever get unset?

@tacaswell tacaswell added this to the v1.4.0 milestone Mar 5, 2014
@vmarkovtsev
Copy link
Contributor Author

Nope. It is used solely in WebAggApplication.start(). Anyway, as seen from the code, the only way to escape from Tornado is hitting ctrl-c, killing the whole application.

cls.started looks like an internal protection against running the loop several times. If we had running() in newer Tornado-s, cls.started would be just redundant.

In my opinion, WebAgg should not run the darn loop itself at all. Instead, there should be a user callback called. But unfortunately it would break the whole matplotlib abstraction.

try:
tornado.ioloop.IOLoop.instance().start()
except KeyboardInterrupt:
print("Server stopped")
Copy link
Member

Choose a reason for hiding this comment

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

I think that we should add cls.started = False here. It may not do any good now but it makes sure that the function never returns with the class in an inconsistent state.

Since Tornado's IOLoop start() blocks until the loop is closed,
cls.started used to be set to True *after* an application actually
stops working.
Revert running() check since it no longer exists in newer versions
of Tornado and will never return back.
@vmarkovtsev
Copy link
Contributor Author

Done

tacaswell added a commit that referenced this pull request Mar 7, 2014
WebAgg: raise WebAggApplication.started flag before blocking
@tacaswell tacaswell merged commit 8e5f304 into matplotlib:master Mar 7, 2014
@mdboom
Copy link
Member

mdboom commented Mar 11, 2014

In my opinion, WebAgg should not run the darn loop itself at all. Instead, there should be a user callback called. But unfortunately it would break the whole matplotlib abstraction.

You can also just not call show and interface with the WebAgg canvas on the figure directly, if you'd rather start an event loop yourself. This is true of all of the backends.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants