Test a case with plotly renderer

[fcollonval]

This adds a test against a notebook that contains a plotly JSON output (from the plotly renderer).

This test works because of #57. But it stands as a proof that we should be really careful with casting.

Moreover, I deeply think that we should skip the code cells outputs cast for performance reason because a plotly plot with millions of points will require looping on the millions of float items for no reasons.

Casting metadata is probably find as the data structure is probably quickly traverse most of the time.

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[davidbrochart]

This test works because of #57. But it stands as a proof that we should be really careful with casting.

I agree that the situation is not ideal, but from the discussion in #55 it seems to be a good compromise. Do you see a better solution?

Moreover, I deeply think that we should skip the code cells outputs cast for performance reason because a plotly plot with millions of points will require looping on the millions of float items for no reasons.

Outputs can have execution_count, so we should at least cast this one. I agree that we should skip the rest.

Casting metadata is probably find as the data structure is probably quickly traverse most of the time.

Yes and I'm afraid we have no choice anyway, because metadata can be custom, so we don't know its structure in advance.

[davidbrochart]

I checked that it indeed works since we have #57. Let's address your other comments in another PR.
Thanks a lot!

[welcome]

Congrats on your first merged pull request in this project! 🎉

Thank you for contributing, we are very proud of you! ❤️

[fcollonval]

Outputs can have execution_count, so we should at least cast this one. I agree that we should skip the rest.

FYI the execution_count is separate key in the code cell dictionary: https://nbformat.readthedocs.io/en/latest/format_description.html#code-cells

We could add something like a escape_keys: Optional[Container] kwarg to cast_all:

   cast_all(cell, float, int, escape_keys={"source", "outputs"})
   
   def cast_all(o, from, to, escape_keys: Optional[Container]=None):
       escape_keys = escape_keys or set()
       ...
       if isinstance(o, dict):
          for k, v in o.items():
              if k not in escape_keys:
                 ...

[davidbrochart]

Also note that a widget output will look like this:

   "outputs": [
    {
     "data": {
      "application/vnd.jupyter.widget-view+json": {
       "model_id": "1da8a6e0747b43e78b9e1d9e59e76067",
       "version_major": 2,
       "version_minor": 0
      },
      "text/plain": [
       "IntSlider(value=0)"
      ]
     },
     "metadata": {},
     "output_type": "display_data"
    }
   ],

With a version_major and a version_minor that we should cast. Maybe arbitrary representations are allowed, which structure we don't know about? In this case we cannot selectively cast.

[davidbrochart]

FYI the execution_count is separate key in the code cell dictionary

I was talking about the execute_count for an output of type execute_result:
https://nbformat.readthedocs.io/en/latest/format_description.html#execute-result