Skip to content

Conversation

AakashGfude
Copy link
Member

This PR implements the feature to execute notebooks in _downloads folder.
To enable this feature, one has to set jupyter_download_nb_execute variable to True in config

@AakashGfude AakashGfude requested a review from mmcky September 5, 2019 02:02
@mmcky mmcky added this to the v0.4.4 milestone Sep 5, 2019
@mmcky
Copy link
Contributor

mmcky commented Sep 5, 2019

thanks @AakashGfude

Copy link
Contributor

@mmcky mmcky left a comment

Choose a reason for hiding this comment

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

thanks @AakashGfude this looks great. If you have tested it I am happy to merge.

@AakashGfude
Copy link
Member Author

@mmcky testing on lecture-source-py worked well

@mmcky
Copy link
Contributor

mmcky commented Sep 5, 2019

@AakashGfude would you mind to test on julia as well. Thanks.

@mmcky mmcky requested a review from arnavs September 5, 2019 06:23
@mmcky
Copy link
Contributor

mmcky commented Sep 5, 2019

@arnavs would you have time to give this branch a try on the julia repo?

@mmcky mmcky added the test label Sep 5, 2019
@arnavs
Copy link
Member

arnavs commented Sep 5, 2019

@mmcky Sure, will give this a go in the next few hours.

@mmcky
Copy link
Contributor

mmcky commented Sep 6, 2019

@AakashGfude when I tested this on the anu workstation it appears the index.html is writen using url links to lectures.quantecon.org rather than relative links as is the case on the master branch. Can you replicate this?

<li><a href="https://lectures.quantecon.org/multi_agent_models/index.html">Multiple Agent Models</a><ul>
<li><a href="https://lectures.quantecon.org/multi_agent_models/schelling.html">Schelling’s Segregation Model</a></li>

should be more like

<li><a href="getting_started_julia/index.html">Getting Started with Julia</a><ul>
<li><a href="getting_started_julia/getting_started.html">Setting up Your Julia Environment</a></li>

@mmcky mmcky added the bug label Sep 6, 2019
Copy link
Contributor

@mmcky mmcky left a comment

Choose a reason for hiding this comment

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

seems to be a bug when forming index.html using this branch.

Copy link
Contributor

@mmcky mmcky left a comment

Choose a reason for hiding this comment

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

Thanks @AakashGfude this is working well for me now.
@arnavs would you mind doing a test on your end for Julia

arnavs added a commit to QuantEcon/lecture-source-jl that referenced this pull request Sep 6, 2019
@arnavs
Copy link
Member

arnavs commented Sep 6, 2019

Just started a test on this and make website. Will approve and merge if it looks good.

@arnavs
Copy link
Member

arnavs commented Sep 6, 2019

Maybe it's just me, but I find two things:

  1. The spinning up of the execution (i.e., Starting notebook execution for website and html conversion(if set in config)...) is taking much longer.

  2. The notebooks in arnavsood@imp:~/research/quantecon/lecture-source-jl/_build/website/jupyter/executed/dynamic_programming are still referring to _static. So then the images are ?s... and _static is still copied.

I set the conf.py in QuantEcon/lecture-source-jl@a8903e4.

@mmcky
Copy link
Contributor

mmcky commented Sep 7, 2019

thanks @arnavs your second point is an issue with Julia and how statics are treated internally in the extension. We have a fix on the aws instance that copies statics to all nested folders at the moment. This PR will fix this in favour of a much more robust implementation of static files: #268

@AakashGfude can you look into the first point raised?

@mmcky
Copy link
Contributor

mmcky commented Sep 9, 2019

@arnavs regarding point #1 -- execution will take twice as long with this option enabled as it is running the html set of notebooks and the download set of notebooks. Is there something else that is taking a lot longer?

@arnavs
Copy link
Member

arnavs commented Sep 9, 2019

@mmcky Maybe that was it. I'm looking through this PR and I don't see anything troublesome, so I'm willing to take the plunge. This will be a great help on the Julia side.

@mmcky mmcky removed the bug label Sep 10, 2019
@mmcky
Copy link
Contributor

mmcky commented Sep 10, 2019

@AakashGfude we need to update this so that if jupyter_download_nb_execute = True then we copy the executed set of notebooks in jupyter/_downloads/executed to jupyter_html/_downloads/ipynb

@AakashGfude
Copy link
Member Author

@AakashGfude we need to update this so that if jupyter_download_nb_execute = True then we copy the executed set of notebooks in jupyter/_downloads/executed to jupyter_html/_downloads/ipynb

@mmcky done. Testing it now

@mmcky
Copy link
Contributor

mmcky commented Sep 10, 2019

thanks @AakashGfude this is looking good now. Once your testing is complete and your happy with it -- let me know and I will merge and release 0.4.4. Please change tag to ready when you're happy.

@mmcky mmcky removed the test label Sep 11, 2019
@mmcky mmcky added the ready label Sep 11, 2019
@mmcky mmcky merged commit f86ecad into master Sep 11, 2019
@mmcky mmcky deleted the execute-downloaded-notebooks branch September 11, 2019 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants