jupyter / nbviewer

nbconvert as a web service: Render Jupyter Notebooks as static web pages
https://nbviewer.jupyter.org
Other
2.19k stars 544 forks source link

Upgrade nbconvert #1015

Closed martinRenou closed 1 year ago

martinRenou commented 1 year ago

Revive https://github.com/jupyter/nbviewer/pull/1003 and merge with https://github.com/jupyter/nbviewer/pull/1014

closes #1003 closes #1014 closes #959

minrk commented 1 year ago

@martinRenou this mostly works now, but the lab template's font-awesome 5 is conflicting with the font-awesome 4 used on the nbviewer page, erasing the icons:

Screen Shot 2022-08-22 at 14 47 05

Not sure if there's a simple fix for that.

martinRenou commented 1 year ago

Thanks a lot for pushing on this @minrk !

but the lab template's font-awesome 5 is conflicting with the font-awesome 4 used on the nbviewer page, erasing the icons:

In Voila we do the following, maybe that will fix the nbviewer case:

<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@fortawesome/fontawesome-free@^5/css/all.min.css" type="text/css" />
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@fortawesome/fontawesome-free@^5/css/v4-shims.min.css" type="text/css" />

See https://github.com/voila-dashboards/voila/blob/2b6694191324109c884dc48cc93073977fdacca6/share/jupyter/voila/templates/lab/index.html.j2#L20

I am not sure where to add this though. I wonder if this should go directly in nbconvert.

minrk commented 1 year ago

We should update to fa5, but this backward-incompatibility is definitely an issue. The shims do work in layout.html (after our own css), but result in a different size than before:

before:

Screen Shot 2022-08-24 at 12 39 30

after:

Screen Shot 2022-08-24 at 12 39 27
Carreau commented 1 year ago

The navbar seem to be missing visually as well.

Screen Shot 2022-10-31 at 09 31 59
Carreau commented 1 year ago

A huge chink of the css issue seem to be with

  {% block ipywidgets %}
    <!-- block ipywidgets notebook.html -->
    <script>
      (function() {
        function addWidgetsRenderer() {
          var mimeElement = document.querySelector('script[type="application/vnd.jupyter.widget-view+json"]');
          var scriptElement = document.createElement('script');
          var widgetRendererSrc = '{{ ipywidgets_base_url }}@jupyter-widgets/html-manager@{{ jupyter_widgets_html_manager_version }}/dist/embed-amd.js';
          var widgetState;

          try {
            widgetState = mimeElement && JSON.parse(mimeElement.innerHTML);

            if (widgetState && (widgetState.version_major < 2 || !widgetState.version_major)) {
              widgetRendererSrc = '{{ ipywidgets_base_url }}jupyter-js-widgets@{{ jupyter_js_widgets_version }}/dist/embed.js';
            }
          } catch(e) {}

          scriptElement.src = widgetRendererSrc;
          document.body.appendChild(scriptElement);
        }

        document.addEventListener('DOMContentLoaded', addWidgetsRenderer);
      }());
    </script>
    <!-- end block ipywidgets notebook.html -->
  {% endblock ipywidgets %}

That dynamically inject css.

minrk commented 1 year ago

@Carreau thanks for picking this up!

Carreau commented 1 year ago

One another big problem is that some notebooks completely fail.

For example, IRuby from the main page:

      File "/Users/bussonniermatthias/miniforge3/envs/nbv/share/jupyter/nbconvert/templates/lab/base.html.j2", line 162, in block 'data_svg'
        {{ output.data['image/svg+xml'] | clean_html }}
      File "src/lxml/html/clean.py", line 562, in lxml.html.clean.Cleaner.clean_html
      File "/Users/bussonniermatthias/miniforge3/envs/nbv/lib/python3.10/site-packages/lxml/html/__init__.py", line 873, in fromstring
        doc = document_fromstring(html, parser=parser, base_url=base_url, **kw)
      File "/Users/bussonniermatthias/miniforge3/envs/nbv/lib/python3.10/site-packages/lxml/html/__init__.py", line 759, in document_fromstring
        value = etree.fromstring(html, parser, **kw)
      File "src/lxml/etree.pyx", line 3254, in lxml.etree.fromstring
      File "src/lxml/parser.pxi", line 1908, in lxml.etree._parseMemoryDocument
    ValueError: Unicode strings with encoding declaration are not supported. Please use bytes input or XML fragments without declaration.
Carreau commented 1 year ago

Most of the reveal issue seem to be coming down to font-size being way to big.

Something like the following in the right place make it bearable.

div.reveal {
    font-size: 10px;

}

Now we do seem to have 2 sets of reveal controls on the page if you look carefully:

Screen Shot 2022-10-31 at 16 15 10
Carreau commented 1 year ago

So there are indeed 2 reveal. One done by nbconvert, and one by nbviewer.

Really this is crazy that we are embedding a full nbconvert with body|safe, that even include DOCTYPE ????. And of course the Reveal from nbconvert has no hooks, so we can't listen to the slide change event to hide the the nbviewer header.

The new nbconverts are really backward in term of reusing the templates...

Carreau commented 1 year ago

I think we should leapfrog nbconvert 6.x and go directly to 7.x, I'm going to create a 1.x branch that is the current main, and suggest main become 2.x.

minrk commented 1 year ago

If that makes it easier, by all means!

Carreau commented 1 year ago

If that makes it easier, by all means!

It's more on the communication side, there will be likely so much change between 1.x and main, that it does make sens to bump major version number. This also let us re-do a 1.x at one point if ever necessary.

I've push a 1.x branch, and I'm going to merge this and keep iterating, once this is merged, main should not be considered stable as a number of notebook likely won't render yet, and reveal is still partially broken .

martinRenou commented 1 year ago

Thanks a lot for pushing on this!