nbviewer icon indicating copy to clipboard operation
nbviewer copied to clipboard

Upgrade nbconvert

Open martinRenou opened this issue 3 years ago • 3 comments

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

closes #1003 closes #1014 closes #959

martinRenou avatar Aug 11 '22 08:08 martinRenou

@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.

minrk avatar Aug 22 '22 13:08 minrk

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.

martinRenou avatar Aug 22 '22 14:08 martinRenou

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

minrk avatar Aug 24 '22 10:08 minrk

The navbar seem to be missing visually as well. Screen Shot 2022-10-31 at 09 31 59

Carreau avatar Oct 31 '22 08:10 Carreau

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.

Carreau avatar Oct 31 '22 09:10 Carreau

@Carreau thanks for picking this up!

minrk avatar Oct 31 '22 12:10 minrk

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 avatar Oct 31 '22 14:10 Carreau

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 avatar Oct 31 '22 15:10 Carreau

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 avatar Nov 01 '22 09:11 Carreau

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.

Carreau avatar Nov 07 '22 09:11 Carreau

If that makes it easier, by all means!

minrk avatar Nov 07 '22 09:11 minrk

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 .

Carreau avatar Nov 07 '22 09:11 Carreau

Thanks a lot for pushing on this!

martinRenou avatar Nov 07 '22 10:11 martinRenou