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

WIP - Lab template #1005

Closed
wants to merge 6 commits into from
Closed

Conversation

SylvainCorlay
Copy link
Member

@SylvainCorlay SylvainCorlay commented Apr 30, 2019

This PR adds an nbconvert template producing the same DOM structure as JupyterLab.

Templates

The goal is to enable the styling of HTML produced with nbconvert with JupyterLab's themes.

Pygments theme

Besides, I am working on a pygments theme using the JupyterLab CSS variables used by the Jupyter CodeMirror theme.

For reference, the pygments stylesheet that produces the same colors as JupyterLab. (This pygments style requires the Pygments PR #814)

from pygments.style import Style
from pygments.token import Keyword, Name, Comment, String, Error, Number, Operator, Generic, Whitespace

class JupyterStyle(Style):
    """
    A Jupyter style for pygments

  --jp-mirror-editor-atom-color
  --jp-mirror-editor-def-color
  --jp-mirror-editor-variable-color
  --jp-mirror-editor-variable-2-color
  --jp-mirror-editor-variable-3-color
  --jp-mirror-editor-punctuation-color
  --jp-mirror-editor-property-color
  --jp-mirror-editor-string-2-color
  --jp-mirror-editor-meta-color
  --jp-mirror-editor-qualifier-color
  --jp-mirror-editor-builtin-color
  --jp-mirror-editor-bracket-color
  --jp-mirror-editor-tag-color
  --jp-mirror-editor-attribute-color
  --jp-mirror-editor-header-color
  --jp-mirror-editor-quote-color
  --jp-mirror-editor-link-color
  --jp-mirror-editor-hr-color
    """

    default_style = ''
    background_color = 'var(--jp-cell-editor-background)'
    highlight_color = 'var(--jp-cell-editor-active-background)'

    styles = {
        Comment:                   'var(--jp-mirror-editor-comment-color)',
        Keyword:                   'bold var(--jp-mirror-editor-keyword-color)',
        Operator:                  'var(--jp-mirror-editor-operator-color)',
        String:                    'var(--jp-mirror-editor-string-color)',
        Number:                    'bold var(--jp-mirror-editor-number-color)',
        Error:                     'var(--jp-mirror-editor-error-color)',
        Name:                      '',
    }

Light theme:

Light

Dark theme:

Dark

cc @mpacer @MSeal @maartenbreddels @ivanov @ellisonbg

@SylvainCorlay SylvainCorlay changed the title Lab template WIP - Lab template Apr 30, 2019
@t-makaro
Copy link
Contributor

Just a note on highlight style. I believe the style is limited by our use of the pygments lexers. I made a style over at nb_pdf_template that mimics the notebook highlight scheme the best it can.

@SylvainCorlay
Copy link
Member Author

Thanks for the pointer @t-makaro. Another goal of my PR is to use jupyterlab's CSS variable so that we can switch themes dynamically and nbconverted HTML can be used within JupyterLab while being consistent with the current Jupyterlab Theme.

@SylvainCorlay SylvainCorlay force-pushed the lab-template branch 3 times, most recently from 8c2c971 to 506a021 Compare May 2, 2019 10:16
@SylvainCorlay
Copy link
Member Author

SylvainCorlay commented May 2, 2019

So this appears to work (and to support JupyterLab themes) but the way to include the JupyterLab CSS is still manual. At the moment, the CSS from the classic notebook (style.min.css) is included in the resources for the jinja template here.

Either I replace basic.tpl and full.tpl with the new JupyterLab versions, or we find a means to make these resources pluggable. Ideally we should do something along the lines of what voila does with templates being installed under a PREFIX/share/jupyter/[voila|nbconvert]/template, but I would like to have a path forward that initially does not require too much change.

@SylvainCorlay SylvainCorlay force-pushed the lab-template branch 4 times, most recently from b622cd1 to ad54d66 Compare May 2, 2019 21:56
@SylvainCorlay
Copy link
Member Author

SylvainCorlay commented May 2, 2019

In the last commit, I replace the style.min.css with the one generated for jlab. This can easily be tested now. This does not include the new pygment theme yet.

The style.min.css CSS bundle that I am using was produced by a webpack bundle. The bundler package may be added to jupyterlab and the bundle uploaded to unpkg.

@SylvainCorlay SylvainCorlay force-pushed the lab-template branch 5 times, most recently from 779fd57 to 950cb1d Compare May 3, 2019 12:47
@SylvainCorlay
Copy link
Member Author

Hey @MSeal @mpacer I would love to hear from you on how I should move forward with this.

@SylvainCorlay SylvainCorlay force-pushed the lab-template branch 2 times, most recently from 9a1539a to 8c47667 Compare May 3, 2019 13:30
@MSeal
Copy link
Contributor

MSeal commented May 3, 2019

To compare the styles from before / after.

lab_theme

I'm not sure what the stance has been for style of nbconvert (in particular the default style) as that was established a bit before I joined. So I think I'd want opinions from devs that have more context on style. It might be surprising in classic notebook to not see the same style exported as you saw on screen for example.

@minrk @willingc I'm not sure if either of you had opinions on the topic or know how non-bug fix style decisions in nbconvert have been made in the past?

@MSeal
Copy link
Contributor

MSeal commented May 3, 2019

Codewise the changes look solid.

@SylvainCorlay
Copy link
Member Author

SylvainCorlay commented May 3, 2019

To compare the styles from before / after.

(Quick note: on your screenshot, "after" is on the left.)

  • Regarding the removal of the top-level bordered div, this could be re-added in Full.tpl.
  • Regarding the slight changes in the syntax highlighting: I am still iterating on the new pygment theme, with the goal of completely mimicking the jupyterlab codemirror theme with the same CSS variables.

@@ -311,7 +311,6 @@ def test_no_prompt(self):
with open("notebook1.html",'r') as f:
text = f.read()
assert "In [" not in text
assert "Out[" not in text
Copy link
Member Author

Choose a reason for hiding this comment

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

Out[ is in fact in the jlab CSS bundle, so I had to remove that test.

@ivanov
Copy link
Member

ivanov commented May 4, 2019

This is looking great, @SylvainCorlay, thank you! Can we copy these files to make a new template so we keep the default as it is right now?

@akhmerov
Copy link
Member

akhmerov commented May 5, 2019

Looks great!

Any idea if it would be possible to hook in the jupyterlab mime rendering machinery? That'd be the only way to display custom rich output iiuc (extensions aren't supposed to render themselves anymore).

@SylvainCorlay
Copy link
Member Author

This is looking great, @SylvainCorlay, thank you! Can we copy these files to make a new template so we keep the default as it is right now?

Thanks. There is a small obstacle which is that beyond the jinja template files, the defaut HTMLExporter provides the CSS of the classic notebook, and this cannot be super-easily overriden. Ideally, the resources required for a template should be specified as part of the template...

I am actually thinking of using a custom HTML exporter in voila so that we can specify such resources in the template directory. The voila configuration directory approach could be upstreamed into nbconvert.

@SylvainCorlay
Copy link
Member Author

Looks great!

Any idea if it would be possible to hook in the jupyterlab mime rendering machinery? That'd be the only way to display custom rich output iiuc (extensions aren't supposed to render themselves anymore).

Actually, we already support rendering live widgets in nbconverted notebooks (see e.g. here or in the ipywidgets documentation, using the rst nbconvert exporter of nbsphinx) but this does not use generic rendermime. This is definitely something we should do, but it is somewhat orthogonal to the choice of template.

@bollwyvl
Copy link
Contributor

bollwyvl commented May 5, 2019 via email

@akhmerov
Copy link
Member

akhmerov commented May 5, 2019

Got it, sorry for the offtopic. I also now found #755, which is the correct place to track plans for supporting mimerender.

@SylvainCorlay
Copy link
Member Author

I opened jupyterlab/jupyterlab#6314 into JupyterLab to produce the CSS bundle. As soon as the package is released, I will be able to reference the unpkg URL in setup.py instead of the current one.

@SylvainCorlay
Copy link
Member Author

@bollwyvl thanks for all the pointers! I was completely unaware of Jive. I would love to hear more about it!

@SylvainCorlay
Copy link
Member Author

SylvainCorlay commented May 7, 2019

I opened jupyterlab/jupyterlab#6314 into JupyterLab to produce the CSS bundle. As soon as the package is released, I will be able to reference the unpkg URL in setup.py instead of the current one.

The JupyterLab CSS bundler has been merged and published to NPM. I have updated this PR to reflect this change. I think we are getting close to a mergeable state.

@SylvainCorlay
Copy link
Member Author

Note: you can now select the dark theme with

jupyter nbconvert --to html --CSSHTMLHeaderPreprocessor.theme='dark' foobar.ipynb

@minrk
Copy link
Member

minrk commented May 7, 2019

@MSeal requested historical perspective: nbconvert HTML style has been defined as "what the notebook does" which is why we bundle the notebook css in nbconvert, and intend to reproduce the DOM of the live notebook.

In that context, with JupyterLab as a peer to nbclassic and its intended replacement, it makes sense to me to add jupyterlab as an output style, and even in the future replace the nbclassic style with jupyterlab as the default, especially if the differences prove uncontroversial.

@MSeal
Copy link
Contributor

MSeal commented May 8, 2019

Thanks for the context.

@SylvainCorlay could you preserve the default as the classic look and make lab an adjacent template then? We'd likely need classic template available even if the default changes in the future.

@jasongrout jasongrout self-requested a review May 8, 2019 18:11
@SylvainCorlay
Copy link
Member Author

@SylvainCorlay could you preserve the default as the classic look and make lab an adjacent template then? We'd likely need classic template available even if the default changes in the future.

This may not be possible merely as an "alternative" jinja template for the HTML exporter, but only as a completely new template exporter (to both change the way the CSS header is composed and the jinja template).

What I am thinking of is to enable the sort of template configuration that we have in voila, where a template is merely a directory in a conf.d, holding all the required resources.

@MSeal
Copy link
Contributor

MSeal commented May 25, 2019

That could work. I'd say use best judgement on how to achieve something mergable in this PR. If we need to hack the exporter a bit to avoid clobbering the current default view I'm ok with that if we have a longer term approach (this could be a change for 6.0?), but we do need a path to preserve the classic export look-n-feel as default for now imo.

@SylvainCorlay
Copy link
Member Author

That could work. I'd say use best judgement on how to achieve something mergable in this PR. If we need to hack the exporter a bit to avoid clobbering the current default view I'm ok with that if we have a longer term approach (this could be a change for 6.0?), but we do need a path to preserve the classic export look-n-feel as default for now imo.

Absolutely. Let's hold this up for a bit more and see if we come up with a nicer approach than a completely new template exporter without too much change.

@SylvainCorlay
Copy link
Member Author

Closing as this was included through another PR.

@MSeal MSeal added this to the no action milestone Sep 8, 2020
@SylvainCorlay SylvainCorlay deleted the lab-template branch September 23, 2020 12:50
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.

8 participants