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

Allow open/interact without plotting anything #1227

Merged
merged 2 commits into from Apr 22, 2015

Conversation

Projects
None yet
3 participants
@chaosphere2112
Contributor

chaosphere2112 commented Apr 20, 2015

Currently, the canvas.open() call doesn't do anything. In developing and testing my GUI parts embedded in VCS, it's really quite handy to have a fully-prepared VCS canvas without actually plotting anything (or plotting something and then calling clear(), which gives the same end result). This PR makes it so you can open the canvas prior to plotting. As a caveat of that, if you call x.open(); x.interact(), it'll now enter interact mode. This is a little goofy, but given the fact that we have some GUI elements that allow for some user interaction with a blank canvas (markers and text buttons, top right), it's not completely ridiculous. I'm happy to scrap the ability to interact() with a blank canvas; it's not particularly useful for me.

All of the tests pass for me with this PR, though there is one test that will hang until manually quit (test_vcs_interact_no_plot.py). If we make it so interact() will fail when there are no plots, that test can remain the same; if we don't, this test will either have to change or be eliminated.

@doutriaux1 @aashish24 @dlonie Opinions?

Sam Fries
@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 21, 2015

@chaosphere2112 do you mean your changes make it fail, right? I have no strong opinion either way whereas we can call x.interact () on a close canvas or not. Just let make sure we update the tests and let the users know about any change.

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented Apr 21, 2015

They make it fail, though it is not flagged by ctest as failing.

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Apr 21, 2015

The test will need to return a non-zero exit status for CTest to pick up the failure. Can you update the test so this will happen?

The changes sounds good. I think being able to call interact on an open, empty canvas makes sense for the reason you mentioned (GUI widgets).

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented Apr 21, 2015

@dlonie The only stumbling block I'm seeing is that if we're going into interactive mode, there's no obvious way to exit it automatically in a test. The only things that come to mind are threads, but I think it would be pretty non-deterministic as to when the test would actually come to an end, because in my experience VTK tends to hog the GIL. Are there any VTK tests for Start that we could emulate?

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Apr 22, 2015

@chaosphere2112 I've done similar things in the past by starting a timer in the eventloop or adding an observer for a signal to close things down.

Does UVCDAT have some sort of signal/slot or event/observer mechanism? If there's some way to register a callback function to be executed when the interaction starts, we could shut it down from there. Otherwise, look at what is managing the UVCDAT interaction event loop and see if there's a way to post a custom event/callback to it.

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented Apr 22, 2015

Oh, yeah, that would be easy; just signed up for the StartEvent event on the interactor before starting it.
Here's the test results:
https://open.cdash.org/buildSummary.php?buildid=3782918

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Apr 22, 2015

Nice! Looks good. Is this ready to go in?

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented Apr 22, 2015

I think it's ready; @doutriaux1 Mind running the test and making sure it works in ubuntu?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Apr 22, 2015

doutriaux1 added a commit that referenced this pull request Apr 22, 2015

Merge pull request #1227 from chaosphere2112/open_interact
Allow open/interact without plotting anything

@doutriaux1 doutriaux1 merged commit 102701b into CDAT:master Apr 22, 2015

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment