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

Added create_static method to Scene class. #117

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tarzzz
Copy link
Contributor

@tarzzz tarzzz commented Mar 30, 2014

It allows you to create static webpages for the visualization.
Added cleanup_static method to remove the static data generated!

To test this branch
modify last line in pydy_examples/illustrative_example_for_viz/illustrative_example.py
from:
scene.display()
to:
scene.create_static()
and run it:
python illustrative_example.py

then cd to static directory generated, and from terminal
$ python -m SimpleHTTPServer
You should be able to see visualization at localhost:8000

static webpages for the visualization.
Added cleanup_static method to remove the static data generated!
@tarzzz
Copy link
Contributor Author

tarzzz commented Mar 30, 2014

@moorepants Please review and merge it.
P.S.: it was already partially implemented in scene class, but was redundant earlier. Now we can have a better use of it :)

@tarzzz tarzzz changed the title Added create_static method to Scene class. It allows you to create Added create_static method to Scene class. Mar 30, 2014
@moorepants
Copy link
Member

This works. No need to open a server. Simply open index.html in your browser.

Gonna add a couple of things before merging.

Safety code is in place in create_static_html and remove_static_html to prevent
unwanted deletions. I also changed the names of the methods to indicate more
precisely what the methods do.
@moorepants
Copy link
Member

These really need tests before merging. Not sure how long that would take. @tarzzz, are you fine with the changes I made.

distutils.dir_util.remove_tree(os.path.join(os.getcwd(), '.pydy_viz'))
dst = os.path.join(os.getcwd(), 'static')

if os.path.exists(dst) and overwrite is False:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am fine with the changes, but I feel we should be writing tests too for the same(because of conditional statements). I can write them and if we are short of time(sending tutorial instructions to students), then we can merge it and tests can be sent in a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

This isn't critical to have for the tutorial, because we already have at least one browser/os combination that works for everyone. But if this was in a new pydy-viz release before the april 2, then we would have it available for backup in case the socket server isn't working for some.

@moorepants
Copy link
Member

@tarzzz You need to get this merged into the pydy repo.

@tarzzz
Copy link
Contributor Author

tarzzz commented Apr 22, 2014

I am fine with the changes, but I feel we should be writing tests too for
the same(lots of conditional statements). I can write them and if we are
short of time(sending tutorial instructions to students), then we can merge
it and tests can be sent in a separate PR.

On Mon, Mar 31, 2014 at 7:37 AM, Jason K. Moore [email protected]:

These really need tests before merging. Not sure how long that would take.
@tarzzz https://github.com/tarzzz, are you fine with the changes I made.


Reply to this email directly or view it on GitHubhttps://github.com//pull/117#issuecomment-39048537
.

@tarzzz
Copy link
Contributor Author

tarzzz commented Apr 22, 2014

Sorry, that was a draft I made some time ago(before PyCon). I am onto it
now.

On Wed, Apr 23, 2014 at 1:22 AM, TARUN GABA [email protected] wrote:

I am fine with the changes, but I feel we should be writing tests too for
the same(lots of conditional statements). I can write them and if we are
short of time(sending tutorial instructions to students), then we can merge
it and tests can be sent in a separate PR.

On Mon, Mar 31, 2014 at 7:37 AM, Jason K. Moore [email protected]:

These really need tests before merging. Not sure how long that would
take. @tarzzz https://github.com/tarzzz, are you fine with the changes
I made.


Reply to this email directly or view it on GitHubhttps://github.com//pull/117#issuecomment-39048537
.

@moorepants
Copy link
Member

@tarzzz

This would be great to have working in the scipy tutorial. Can you get this working in the new pydy repo?

We plan to send out final instructions by June 23rd so it needs to be done sometime before that.

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.

2 participants