Skip to content

Updated offline mode #348

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

Merged
merged 12 commits into from
Nov 17, 2015
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,20 @@ This project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

## [1.9.0] - 2015-11-15
- Previously, using plotly offline required a paid license.
No more: `plotly.js` is now shipped inside this package to allow
unlimited free use of plotly inside the ipython notebook environment.
The `plotly.js` library that is included in this package is free,
open source, and maintained independently on GitHub at
[https://github.com/plotly/plotly.js](https://github.com/plotly/plotly.js).
- The `plotly.js` bundle that is required for offline use is no longer downloaded
and installed independently from this package: `plotly.offline.download_plotlyjs`
is **deprecated**.
- New versions of `plotly.js` will be tested and incorporated
into this package as new versioned pip releases;
`plotly.js` is not automatically kept in sync with this package.

## [1.8.12] - 2015-11-02
- *Big data* warning mentions `plotly.graph_objs.Scattergl` as possible solution.

Expand Down
325 changes: 210 additions & 115 deletions plotly/graph_reference/default-schema.json

Large diffs are not rendered by default.

88 changes: 37 additions & 51 deletions plotly/offline/offline.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,40 +8,29 @@
import json
import os
import uuid

import requests
import warnings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐄 mind flipping these import statements? We typically do from ... after import ....

from pkg_resources import resource_string

from plotly import session, tools, utils
from plotly.exceptions import PlotlyError

PLOTLY_OFFLINE_DIRECTORY = plotlyjs_path = os.path.expanduser(
os.path.join(*'~/.plotly/plotlyjs'.split('/')))
PLOTLY_OFFLINE_BUNDLE = os.path.join(PLOTLY_OFFLINE_DIRECTORY,
'plotly-ipython-offline-bundle.js')


__PLOTLY_OFFLINE_INITIALIZED = False


def download_plotlyjs(download_url):
if not os.path.exists(PLOTLY_OFFLINE_DIRECTORY):
os.makedirs(PLOTLY_OFFLINE_DIRECTORY)

res = requests.get(download_url)
res.raise_for_status()
warnings.warn('''
`download_plotlyjs` is deprecated and will be removed in the
next release. plotly.js is shipped with this module, it is no
longer necessary to download this bundle separately.
''', DeprecationWarning)
pass
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, if it's just deprecated, why are we getting rid of this function? Shouldn't we do that in 2.x?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yhea, you're right.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't really keep this functionality in here and include a shipped version of plotly.js. I can either remove this function, or keep the warning in case someone finds an old reference of this function. In either case, the purpose of this function is redundant - a pass or performing the download have the same affect.
Keep the function, or keep the warning?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, nbd. I think it's alright to leave it as-is then.


with open(PLOTLY_OFFLINE_BUNDLE, 'wb') as f:
f.write(res.content)

print('\n'.join([
'Success! Now start an IPython notebook and run the following ' +
'code to make your first offline graph:',
'',
'import plotly',
'plotly.offline.init_notebook_mode() '
'# run at the start of every ipython notebook',
'plotly.offline.iplot([{"x": [1, 2, 3], "y": [3, 1, 6]}])'
]))
def get_plotlyjs():
path = os.path.join('offline', 'plotly.min.js')
plotlyjs = resource_string('plotly', path).decode('utf-8')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the future, I'd also like to ship an un-synced version of the plot-schema.json that corresponds to this particular plotly.min.js and validate offline mode against that plot-schema.json.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little confused how this is working? I thought you needed a line in setup.py telling the setup script to include the .js file in the package information. Like here.

I think you need a 'offline/*.js' item in that list?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, interesting. OK, I'll add it there.

return plotlyjs


def init_notebook_mode():
Expand All @@ -55,24 +44,20 @@ def init_notebook_mode():
raise ImportError('`iplot` can only run inside an IPython Notebook.')
from IPython.display import HTML, display

if not os.path.exists(PLOTLY_OFFLINE_BUNDLE):
raise PlotlyError('Plotly Offline source file at {source_path} '
'is not found.\n'
'If you have a Plotly Offline license, then try '
'running plotly.offline.download_plotlyjs(url) '
'with a licensed download url.\n'
"Don't have a Plotly Offline license? "
'Contact [email protected] learn more about licensing.\n'
'Questions? [email protected].'
.format(source_path=PLOTLY_OFFLINE_BUNDLE))

global __PLOTLY_OFFLINE_INITIALIZED
__PLOTLY_OFFLINE_INITIALIZED = True
display(HTML('<script type="text/javascript">' +
open(PLOTLY_OFFLINE_BUNDLE).read() + '</script>'))
# ipython's includes `require` as a global, which
# conflicts with plotly.js. so, unrequire it.
'require=requirejs=define=undefined;' +
'</script>' +
'<script type="text/javascript">' +
get_plotlyjs() +
'</script>'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:( I'm not super familiar with the repercussions of this. Is this a safe thing to do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, I've been doing it since the release of offline.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 perfect. thanks.



def iplot(figure_or_data, show_link=True, link_text='Export to plot.ly'):
def iplot(figure_or_data, show_link=True, link_text='Export to plot.ly',
validate=True):
"""
Draw plotly graphs inside an IPython notebook without
connecting to an external server.
Expand All @@ -90,6 +75,11 @@ def iplot(figure_or_data, show_link=True, link_text='Export to plot.ly'):
of the chart that will export the chart to
Plotly Cloud or Plotly Enterprise
link_text (default='Export to plot.ly') -- the text of export link
validate (default=True) -- validate that all of the keys in the figure
are valid? omit if your version of plotly.js
has become outdated with your version of
graph_reference.json or if you need to include
extra, unnecessary keys in your figure.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like it could potentially be a bad user experience. Might we be better off just forcing users to keep their plotly.min.js file updated?

Rationale:

  • say there is a conflict between your local version and plotly, when you finally want to push up a file... it would break, which is pretty confusing!
  • plotly.js tries very hard to provide backwards compatibility, I think we might be better off just allowing these rare instances to raise errors.
  • if we do have an out of sync plotly.min.js, then we also need to provide an out of sync plot-schema, i feel like we're potentially creating tough environments to recreate for debugging.

Would it be feasible to get away from this pattern?

I like how the graph_reference is setup, where we have the following:

  • a method to download the most recent
  • a copy that lives in a users local ~ directory
  • a versioned default (for offline users and such)

The thing I don't like is that the initial download is frustratingly slow. Is there any way we can provide this without the long import lag?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's my rational:

  • It's easier for users to understand what's going on. It'll be very clear which version of plotly.js they are using. Upgrading the python client is a natural action taken by users who want to new functionality. Upgrading plotly.js is separately is another process to teach and document.
  • We can formally introduce new functionality as new releases. A new chart type in plotly.js will trigger new chart type releases in Python, and update the changelog accordingly.
  • We can test the integration of R and Python with plotly.js independently, and be confident about our releases (and documentation)
  • It's easier for us to document. If a user views our docs, they can check their version of the python client against the documentation's versions. If something doesn't work with their chart, it's easier for them to rule-out it being an issue with an incompatible version.
  • It's easier for IT depts to manage - version x of the package will work exactly like every other user's version x.
  • It's easier for IT depts to distribute. If this package is outdated from plotly.js and requires an automatic background upgrade, then the IT dept has to make sure that the machines that are using plotly are going to be internet connected the first time that they make a chart and that the network won't block cdn.plot.ly. And if they can't ensure that, they'll have to add some script that will do that plotly.js installation in addition to the pip install

I think by maintaining and versioning everything under pip, we can have a more predictable installation process.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sold. okie dokie. so then /offline will now have:

  • offline/plotly.min.js
  • offline/plot-schema.json

And somehow we'll need to route validation through the appropriate plot-schema.json?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Won't get to this until the next version though. For now, offline.iplot will have slightly more validation than it used to (none), but not quite as much as it needs to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, sounds good!


Example:
```
Expand All @@ -112,15 +102,10 @@ def iplot(figure_or_data, show_link=True, link_text='Export to plot.ly'):
raise ImportError('`iplot` can only run inside an IPython Notebook.')

from IPython.display import HTML, display
if isinstance(figure_or_data, dict):
data = figure_or_data['data']
layout = figure_or_data.get('layout', {})
else:
data = figure_or_data
layout = {}
figure = tools.return_figure_from_figure_or_data(figure_or_data, validate)

width = layout.get('width', '100%')
height = layout.get('height', 525)
width = figure.get('layout', {}).get('width', '100%')
height = figure.get('layout', {}).get('height', 525)
try:
float(width)
except (ValueError, TypeError):
Expand All @@ -136,11 +121,13 @@ def iplot(figure_or_data, show_link=True, link_text='Export to plot.ly'):
width = str(width) + 'px'

plotdivid = uuid.uuid4()
jdata = json.dumps(data, cls=utils.PlotlyJSONEncoder)
jlayout = json.dumps(layout, cls=utils.PlotlyJSONEncoder)
jdata = json.dumps(figure.get('data', []), cls=utils.PlotlyJSONEncoder)
jlayout = json.dumps(figure.get('layout', {}), cls=utils.PlotlyJSONEncoder)

if show_link is False:
link_text = ''
config = {}
config['showLink'] = show_link
config['linkText'] = link_text
jconfig = json.dumps(config)

plotly_platform_url = session.get_session_config().get('plotly_domain',
'https://plot.ly')
Expand All @@ -156,18 +143,17 @@ def iplot(figure_or_data, show_link=True, link_text='Export to plot.ly'):
'<script type="text/javascript">'
'window.PLOTLYENV=window.PLOTLYENV || {};'
'window.PLOTLYENV.BASE_URL="' + plotly_platform_url + '";'
'Plotly.LINKTEXT = "' + link_text + '";'
'</script>'
))

script = '\n'.join([
'Plotly.plot("{id}", {data}, {layout}).then(function() {{',
'Plotly.plot("{id}", {data}, {layout}, {config}).then(function() {{',
' $(".{id}.loading").remove();',
'}})'
]).format(id=plotdivid,
data=jdata,
layout=jlayout,
link_text=link_text)
config=jconfig)

display(HTML(''
'<div class="{id} loading" style="color: rgb(50,50,50);">'
Expand Down
91 changes: 91 additions & 0 deletions plotly/offline/plotly.min.js

Large diffs are not rendered by default.

Empty file.
25 changes: 0 additions & 25 deletions plotly/tests/test_core/test_offline/test_offline.py

This file was deleted.

36 changes: 8 additions & 28 deletions plotly/tests/test_optional/test_offline/test_offline.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,21 @@
"""
from __future__ import absolute_import

import os
from nose.plugins.attrib import attr
from nose.tools import raises
from unittest import TestCase

import plotly
from plotly.exceptions import PlotlyError

dummy_js_url = ('https://gist.githubusercontent.com/chriddyp/'
'f40bd33d1eab6f0715dc/raw/'
'24cd2e4e62ceea79e6e790b3a2c94cda63510ede/'
'test.js')


class PlotlyOfflineTestCase(TestCase):
def _remove_plotlyjs(self):
try:
os.remove(plotly.offline.offline.PLOTLY_OFFLINE_BUNDLE)
except OSError:
pass

def test_no_errors_are_raised_when_initializing_offline_mode(self):
self._remove_plotlyjs()
plotly.offline.download_plotlyjs(dummy_js_url)
plotly.offline.init_notebook_mode()
plotly.offline.iplot([{'x': [1, 2, 3]}])
def setUp(self):
plotly.offline.offline.__PLOTLY_OFFLINE_INITIALIZED = False

@attr('slow')
@raises(PlotlyError)
def test_calling_iplot_before_initializing_raises_an_error(self):
self._remove_plotlyjs()
plotly.offline.download_plotlyjs(dummy_js_url)
plotly.offline.iplot([{'x': [1, 2, 3]}])
@raises(plotly.exceptions.PlotlyError)
def test_iplot_doesnt_work_before_you_call_init_notebook_mode(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wondering if we want to keep these since we're only deprecating this functionality, not removing it yet?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plotly.offline.iplot([{}])

@raises(PlotlyError)
def test_initializing_before_downloading_raises_an_error(self):
self._remove_plotlyjs()
def test_iplot_works_after_you_call_init_notebook_mode(self):
plotly.tools._ipython_imported = True
plotly.offline.init_notebook_mode()
plotly.offline.iplot([{}])
2 changes: 1 addition & 1 deletion plotly/version.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = '1.8.12'
__version__ = '1.9.0'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,6 @@ def readme():
'plotly/matplotlylib',
'plotly/matplotlylib/mplexporter',
'plotly/matplotlylib/mplexporter/renderers'],
package_data={'plotly': ['graph_reference/*.json', 'widgets/*.js']},
package_data={'plotly': ['graph_reference/*.json', 'widgets/*.js', 'offline/*.js']},
install_requires=['requests', 'six', 'pytz'],
zip_safe=False)