Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
DinoV opened this issue Apr 10, 2017 · 9 comments
Open

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

DinoV opened this issue Apr 10, 2017 · 9 comments

Comments

@DinoV
Copy link

DinoV commented Apr 10, 2017

Description

The F# kernel should support returning the Plotly mime type:
https://github.com/plotly/plotly.py/blob/f65724f06b894a5db94245ee4889c632b887d8ce/plotly/offline/offline.py#L347
plotly/plotly.py#562 (comment)

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
Copy link
Member

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
Copy link
Member

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
Copy link

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

@cgravill
Copy link
Member

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

@xperiandri
Copy link

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

@xperiandri
Copy link

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

@cgravill
Copy link
Member

cgravill commented Sep 1, 2017

@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
Copy link
Member

cgravill commented Sep 5, 2017

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
Copy link
Member

cgravill commented Nov 1, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants