search

nteract / hydrogen

:atom: Run code interactively, inspect data, and plot. All the power of Jupyter kernels, inside your favorite text editor.
https://nteract.gitbooks.io/hydrogen/
MIT License
3.91k stars 335 forks source link

Built-in Plotly.js version #1838

Open nicolaskruchten opened 4 years ago

nicolaskruchten commented 4 years ago

:memo: Summary

👋 Hi, I'm Nicolas Kruchten, VP of Product for Plotly!

Right now Hydrogen bundles @nteract/plotly v1.48.3 (which actually contains plotly.js v1.47.1) but this package is now deprecated in favour of plotly.js-dist (which is currently at v1.51.2).

My feature request is to depend on the official plotly.js dist bundle and update it regularly, instead of depending on an older repackaged bundle :)

:checkered_flag: Motivation

As plotly.js development advances, new features are made available in Plotly.py. If users use recent versions of Plotly.py in Hydrogen but the underlying plotly.js bundle is out of date, they will not be able to use new features like Plotly Express, advanced maps, treemaps, imshow etc :)

:runner: Approach

In principle this is a question of changing a couple of lines in package.json and here: https://github.com/nteract/hydrogen/blob/ff4efdd3276f34275083d6fbf5c6c826ef99347b/lib/components/result-view/plotly.js#L66

Note: it seems this file is intended to behave like @nteract/transform-plotly (per https://github.com/nteract/hydrogen/blob/ff4efdd3276f34275083d6fbf5c6c826ef99347b/lib/components/result-view/plotly.js#L10) and that package now depends on plotly.js-dist :)

:anchor: Drawbacks

Nothing comes to mind.

:thinking: Alternatives

Nothing comes to mind.

:question: Unresolved questions

Not from me!

wadethestealth commented 4 years ago

@nicolaskruchten Would you like to create a PR? I believe this change is good.

On a side note, I wish we could depend on transform-plotly, but due to #1653 I had to add a special function button since atom doesn't allow download links by default and it was simpler to override the feature here locally. Maybe an ability to specify download method would be nice for plotly.js since not every node context allows link clicking for download.

nicolaskruchten commented 4 years ago

Sure, I'll PR this in, if you like the idea.

Re #1653 hmm I see the problem. We could just use the config to disable this little button, which actually doesn't work all that well to be honest, as by default it has a fixed (and usually inappropriate!) size. That would allow you to use transform-plotly.

wadethestealth commented 4 years ago

I think it's extremely important we have download functionality though especially because atom can't right click to open/save a ploty graph and I think it's important for data scientist to be able to download their graphs for embedding of visual evidence purposes. Also thank you for bringing this issue to our attention.

nicolaskruchten commented 4 years ago

Ok in that case I’ll include a workaround in my PR that auto sizes the downloaded plot :)