IfSharp icon indicating copy to clipboard operation
IfSharp copied to clipboard

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

Open DinoV opened this issue 8 years ago • 9 comments

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

DinoV avatar Apr 10 '17 20:04 DinoV

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 avatar Apr 11 '17 14:04 cgravill

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.

cgravill avatar Jul 16 '17 18:07 cgravill

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

xperiandri avatar Aug 30 '17 19:08 xperiandri

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

cgravill avatar Aug 30 '17 20:08 cgravill

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

xperiandri avatar Aug 30 '17 22:08 xperiandri

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

xperiandri avatar Aug 31 '17 19:08 xperiandri

@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 avatar Sep 01 '17 13:09 cgravill

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 avatar Sep 05 '17 09:09 cgravill

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

cgravill avatar Nov 01 '17 09:11 cgravill