Pr240 fixing static directory creation issue #242
Conversation
|
Please review #241 first so that I can merge it with new server and test it before merging. |
|
There is one issue now that we are not keeping previous scene files. When we run all cells again in Ipython notebook, it checks that the code has not been edited and thus, does not rerun it, as a result no new json file is generated while the old one has been deleted. This causes scene.display_ipython() to raise error. |
| @system.setter | ||
| def system(self, new_system): | ||
|
|
||
| if new_system is not None and not isinstance(new_system, System): |
There was a problem hiding this comment.
only need to check isinstance
There was a problem hiding this comment.
I had it that way at first and the tests errored. On construction of the instance system maybe be set to None, so this will error for no reason. Passing in None or a valid System are the two valid options.
There was a problem hiding this comment.
I have changed it back to this as test with system=None were failing.
|
|
||
| Notes | ||
| ===== | ||
| The user is allow to supply either system or time, constants, |
|
Yes, that fixed the issue with ipython. Now it is working fine. A directory pydy-resources is created in current directory which consists of json files and static data. |
|
Do you think this would work for cross platform hidden directories: http://stackoverflow.com/questions/25432139/python-cross-platform-hidden-file |
| self.create_static_html() | ||
| server = Server(scene_file=self._scene_json_file) | ||
| self._generate_json() | ||
| server = Server(scene_file=self._scene_json_file, directory=self._static_url) |
There was a problem hiding this comment.
It isn't clear to me when self._static_url is created.
There was a problem hiding this comment.
I am assigning it here: https://github.com/pydy/pydy/pull/242/files#diff-b43b925b1507940800c850f885764b51R139
There was a problem hiding this comment.
Found it in __init__, see my note there.
There was a problem hiding this comment.
And why is this called a url?
|
Normal visualizations can work for cross-platform hidden directories but again Ipython notebook will not work. |
| for k, v in default_kwargs.items(): | ||
| setattr(self, k, v) | ||
|
|
||
| self._static_url = os.path.join(os.getcwd(), "pydy-resources") |
There was a problem hiding this comment.
I'm not sure that this is the best place to create the path to this dir. The user coudl theorectically change directory in between the calls to __init__() and display().
I would make a class level attribute called pydy_dir or something. And then in the display() methods you should do this os.path.join.
| @@ -462,8 +605,8 @@ def remove_static_html(self, force=False): | |||
|
|
|||
| def display(self): | |||
| """Displays the scene in the default webbrowser.""" | |||
| self.create_static_html() | |||
| server = Server(scene_file=self._scene_json_file) | |||
| self._generate_json() | |||
There was a problem hiding this comment.
Don't you have to supply the directory kwarg here, other wise the json files will not be copied into the _static_url directory.
Got it. Thanks. |
|
Your patch fails linting requirements. Check the Travis build log for details. |
|
Closing in favor of #245. |
commit message.
in the unit test.)
nosetests) and on Travis CI.pylint, to check your code)
Notes.
follow deprecation cycles.)