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

updating documentation #126

Merged
merged 4 commits into from
May 14, 2019
Merged

Conversation

choldgraf
Copy link
Contributor

This updates a bit of the documentation that's contained in the readthedocs folder...it doesn't try to do too much other than adding a few new pieces of content. There's probably some information in there that isn't quite correct, so please feel free to give comments and/or edits!

cc @SylvainCorlay who I chatted about this with!

Copy link
Member

@SylvainCorlay SylvainCorlay left a comment

Choose a reason for hiding this comment

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

Thanks!

Indeed, templates and nbconvert options are probably the most important extension points to document.

docs/source/customize.rst Outdated Show resolved Hide resolved

In order to create your own template, first familiarize yourself with **Jinja**,
**HTML**, and **CSS**. Each of these is used in creating custom templates.
For reference, see the TODO: link to nbconvert documentation as well as the
Copy link
Member

Choose a reason for hiding this comment

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

This may be intereting to specify how passing extra NbConvert options to the command line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't currently possible though, right?

@choldgraf
Copy link
Contributor Author

choldgraf commented May 2, 2019

Quick question: I believe that I've got a custom Voila template properly created / located, but Voila doesn't seem to be using the custom template that I've created:

Looking at the logs, it seems like even though the custom template path is there (it's in mytemplate):

[Voila] nbconvert template paths: ['/home/choldgraf/.local/share/jupyter/voila/template/mytemplate/nbconvert_templates', '/home/choldgraf/anaconda/envs/dev/share/jupyter/voila/template/default/nbconvert_templates']

Once the actual template is applied, the custom template path comes after the default template path.

[Voila] Attempting to load template voila.tpl
[Voila]     template_path: /home/choldgraf/anaconda/envs/dev/share/jupyter/voila/template/default/nbconvert_templates:/home/choldgraf/.local/share/jupyter/voila/template/mytemplate/nbconvert_templates:/home/choldgraf/anaconda/envs/dev/share/jupyter/voila/template/default/nbconvert_templates

Any idea why this would happen?

(also see latest push for the commands that I used, more or less)

@SylvainCorlay
Copy link
Member

@choldgraf thanks! Let me know if you think this is ready!

@choldgraf
Copy link
Contributor Author

choldgraf commented May 7, 2019

@SylvainCorlay see my previous comment: #126 (comment) I'm currently blocking on actually getting the custom template to be used. I haven't had time yet to dig into the code and figure out what's going wrong, but will try giving it a shot soon

README.md Outdated
@@ -1,7 +1,7 @@
# Voila

[![Documentation](http://readthedocs.org/projects/voila/badge/?version=latest)](https://voila.readthedocs.io/en/latest/?badge=latest)
[![Binder](https://img.shields.io/badge/launch-binder-brightgreen.svg)](https://mybinder.org/v2/gh/QuantStack/voila/stable?urlpath=voila/tree/notebooks)
[![Binder](https://mybinder.org/badge_logo.svg)](https://mybinder.org/v2/gh/QuantStack/voila/master?urlpath=voila%2Ftree%2Fnotebooks)
Copy link
Member

Choose a reason for hiding this comment

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

We prefer using the stable branch for the binder rather than voila. stable is only updated upon releases.

@@ -0,0 +1,2 @@
sphinx_copybutton
alabaster_jupyterhub
Copy link
Member

Choose a reason for hiding this comment

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

Do we need alabaster_jupyterhub?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, it's copypasta, I'll remove it

@SylvainCorlay
Copy link
Member

Quick question: I believe that I've got a custom Voila template properly created / located, but Voila doesn't seem to be using the custom template that I've created:

How are you invoking the template?

@choldgraf
Copy link
Contributor Author

voila path/to/ntbk.ipynb --template=mytemplate if I recall correctly, about to board a flight so will give it another go while I'm in the air

@choldgraf
Copy link
Contributor Author

Digging into it a little bit more, it seems like something weird is going on w/ my template paths when I run voila.

The problem seems to be that the list nbconvert_template_paths changes from
the initialization of voila to the execution of the "get" request. e.g.
if I put

print(self.nbconvert_template_paths)

When the VoilaHandler is initialized in handler.py (here: https://github.com/QuantStack/voila/blob/master/voila/handler.py#L22) I get the following list:

['/home/choldgraf/.local/share/jupyter/voila/template/mytemplate/nbconvert_templates',
'/home/choldgraf/anaconda/envs/dev/share/jupyter/voila/template/default/nbconvert_templates']

However if I then do the same thing within get( (here: https://github.com/QuantStack/voila/blob/master/voila/handler.py#L29), I get:

['/home/choldgraf/anaconda/envs/dev/share/jupyter/voila/template/default/nbconvert_templates',
'/home/choldgraf/.local/share/jupyter/voila/template/mytemplate/nbconvert_templates',
'/home/choldgraf/anaconda/envs/dev/share/jupyter/voila/template/default/nbconvert_templates']

So it seems like somehow that list is changing, and in the second case, the default path comes before the mytemplate path. Any idea why that would be?

@SylvainCorlay
Copy link
Member

Thanks @choldgraf I have left a couple of inline comments (regarding the binder and the requirements file). Let me know when this is fixed an I am happy to merge.

@choldgraf
Copy link
Contributor Author

@SylvainCorlay see my latest comment for the blocker on this (#126 (comment))

There are a few challenges for me:

  • I can't get a custom template to actually be used (see updating documentation #126 (comment))
  • I can't get voila to install in development mode (there are no developer install instructions and running pip install -e . hangs after "Successfully compile 3 files with Babel"

So...if you want I can just spot-check your suggestions and then figure the other stuff out later, but I can't tell if this documentation actually works or not

docs/doc-requirements.txt Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@SylvainCorlay
Copy link
Member

I can't get a custom template to actually be used (see #126 (comment))

This is really surprising as we have been making several different voila templates. Will look into this.

@maartenbreddels
Copy link
Member

@choldgraf that duplicate template path was a bug that is fixed, the new release (0.1) should fix that.

@SylvainCorlay
Copy link
Member

@choldgraf that duplicate template path was a bug that is fixed, the new release (0.1) should fix that.

Correction: we published 0.0.10.

I am waiting on the jupyterlab theme for 0.1.

@choldgraf
Copy link
Contributor Author

ok lemme upgrade and see if that fixes it

@choldgraf
Copy link
Contributor Author

it worked for me! wohoo!

@choldgraf
Copy link
Contributor Author

ok I'm +1 on merging this whenever y'all are happy with it - happy to continue spot-checking docs in other PRs as well as I continue to learn the codebase better (and if I can ever get the damn thing installed in developer mode)

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

3 participants