fsprojects / IfSharp

F# for Jupyter Notebooks
Other
441 stars 71 forks source link

Support plotly mime type (application/vnd.plotly.v1+json) #142

Open DinoV opened 7 years ago

DinoV commented 7 years ago

Description

The F# kernel should support returning the Plotly mime type: https://github.com/plotly/plotly.py/blob/f65724f06b894a5db94245ee4889c632b887d8ce/plotly/offline/offline.py#L347 https://github.com/plotly/plotly.py/pull/562#issuecomment-245078317

Supporting this will allow notebooks to be previewed with plotly graphs in a safe way. Because this is not currently supported the graphs are rendered using HTML + JavaScript which cannot be safely rendered.

Repro steps

See this notebook: https://notebooks.azure.com/library/HorsesForCourses/html/Ages.ipynb It includes a Plotly graph but the rendering is all HTML. But if you look at a Python notebook:

https://notebooks.azure.com/dino/libraries/b4hsjDh0TBo/html/Using%20Plotly%20in%20Jupyter%20Notebooks%20on%20Microsoft%20Azure.ipynb

You can see with this Python notebook that includes the extra mime type response and that the chart renders:

https://notebooks.azure.com/dino/libraries/kphrQvkqZ6Y/html/Using%20Plotly%20in%20Jupyter%20Notebooks%20on%20Microsoft%20Azure.ipynb

cgravill commented 7 years ago

Thanks, we'd definitely like to support such modes. Is this particular for nteract like tools or would it help with persisted charts more generally? We've had an ongoing annoyance that when you reload a notebook it can be missing previous Plotly charts. I'd suspected it was a a question of safe rendering but hadn't had a chance to look into it.

cgravill commented 7 years ago

This is probably where it needs a change: https://github.com/fsprojects/IfSharp/blob/master/src/IfSharp.Kernel/helpers/XPlot.Plotly.fsx#L11 there's some adjustments needed to return a bundle of responses.

xperiandri commented 6 years ago

If this is such a simple step will it be enough to make a pull request?

cgravill commented 6 years ago

Sure, I'll very happily merge a pull request that fixes any of the issues.

xperiandri commented 6 years ago

@DinoV, what do you think? Will this change be enough to fix the issue?

xperiandri commented 6 years ago

@cgravill, is there a manual of how to at least restore dependencies? I opened a project and everything is highlighted with red line!

cgravill commented 6 years ago

@xperiandri dependencies are managed using Paket. There are lots of ways to use it but this will do it https://fsprojects.github.io/Paket/getting-started.html#Installing-dependencies note only the bootstraper is checked in https://fsprojects.github.io/Paket/bootstrapper.html

btw we could use auto-restore (https://fsprojects.github.io/Paket/paket-auto-restore.html), which I use on projects to make things easier on people who prefer Visual Studio but perhaps less convenient for people who prefer to manage their dependencies elsewhere and don't use Visual Studio.

cgravill commented 6 years ago

I've investigated this more. There are two parts, where the Plotly library is injected, either entirely or via CDN, and then the inject of charts using the library.

@DinoV for your Python sample I get: The error is tracked by id: 7b337489-596f-4c20-9904-3f76fb5f3cb5

So I created my own sample here: https://notebooks.azure.com/cgravill/libraries/python/html/aaa.ipynb

In the ipynb it has the script with both mime types

"data": {
      "text/html": [
       "<script>requirejs.config({paths: { 'plotly': ['https://cdn.plot.ly/plotly-latest.min']},});if(!window.Plotly) {{require(['plotly'],function(plotly) {window.Plotly=plotly;});}}</script>"
      ],
      "text/vnd.plotly.v1+html": [
       "<script>requirejs.config({paths: { 'plotly': ['https://cdn.plot.ly/plotly-latest.min']},});if(!window.Plotly) {{require(['plotly'],function(plotly) {window.Plotly=plotly;});}}</script>"
      ]
     }

do you trust the content of "text/vnd.plotly.v1+html" and inject it into the users page? Do you check it against a whitelist or some other way ensure safety?

For historical reasons the F# kernel injects the whole script - corresponds to init_notebook_mode(connected=False) but we can potentially adjust.

Then the chart itself with "application/vnd.plotly.v1+json" has a double copy of the chart rendering:

     "data": {
      "application/vnd.plotly.v1+json": {
       "data": [
        {
         "marker": {
          "color": "red",
          "size": "10",
          "symbol": 104
         },
         "mode": "markers+lines",
         "name": "1st Trace",
         "text": [
          "one",
          "two",
          "three"
         ],
         "type": "scatter",
         "x": [
          1,
          2,
          3
         ],
         "y": [
          4,
          5,
          6
         ]
        }
       ],
       "layout": {
        "title": "First Plot",
        "xaxis": {
         "title": "x1"
        },
        "yaxis": {
         "title": "x2"
        }
       }
      },
      "text/html": [
       "<div id=\"ea3de543-d6ff-4095-b234-543fa2148828\" style=\"height: 525px; width: 100%;\" class=\"plotly-graph-div\"></div><script type=\"text/javascript\">require([\"plotly\"], function(Plotly) { window.PLOTLYENV=window.PLOTLYENV || {};window.PLOTLYENV.BASE_URL=\"https://plot.ly\";Plotly.newPlot(\"ea3de543-d6ff-4095-b234-543fa2148828\", [{\"type\": \"scatter\", \"x\": [1, 2, 3], \"y\": [4, 5, 6], \"marker\": {\"color\": \"red\", \"symbol\": 104, \"size\": \"10\"}, \"mode\": \"markers+lines\", \"text\": [\"one\", \"two\", \"three\"], \"name\": \"1st Trace\"}], {\"title\": \"First Plot\", \"xaxis\": {\"title\": \"x1\"}, \"yaxis\": {\"title\": \"x2\"}}, {\"showLink\": true, \"linkText\": \"Export to plot.ly\"})});</script>"
      ],
      "text/vnd.plotly.v1+html": [
       "<div id=\"ea3de543-d6ff-4095-b234-543fa2148828\" style=\"height: 525px; width: 100%;\" class=\"plotly-graph-div\"></div><script type=\"text/javascript\">require([\"plotly\"], function(Plotly) { window.PLOTLYENV=window.PLOTLYENV || {};window.PLOTLYENV.BASE_URL=\"https://plot.ly\";Plotly.newPlot(\"ea3de543-d6ff-4095-b234-543fa2148828\", [{\"type\": \"scatter\", \"x\": [1, 2, 3], \"y\": [4, 5, 6], \"marker\": {\"color\": \"red\", \"symbol\": 104, \"size\": \"10\"}, \"mode\": \"markers+lines\", \"text\": [\"one\", \"two\", \"three\"], \"name\": \"1st Trace\"}], {\"title\": \"First Plot\", \"xaxis\": {\"title\": \"x1\"}, \"yaxis\": {\"title\": \"x2\"}}, {\"showLink\": true, \"linkText\": \"Export to plot.ly\"})});</script>"
      ]
     }

I have a branch with a prototype of sending multiple mime https://github.com/fsprojects/IfSharp/tree/Multiple-XPlot-content but need to understand what it is that needs sending to safely display.

cgravill commented 6 years ago

By way of update, the Plotly folks are making improvements to this then hopefully we can get it sorted here.