Skip to content

Animations/Frames in Python API #584

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 27 commits into from
Nov 26, 2016
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
cb4552c
iplot in offline works with frames
Kully Oct 18, 2016
b4c3953
updated js version in plotly offline
Kully Oct 19, 2016
54c1386
added Andrew's #586 PR
Kully Oct 20, 2016
d19c6ca
added some print statements to check progresses
Kully Oct 21, 2016
efa390e
__init__.py now accepts create_animations which lives in plotly.py --…
Kully Oct 28, 2016
eb57218
rewrote create_animations to take frames - still beta
Kully Nov 3, 2016
5482b53
merged master offline.py to local - changed call signature in one func
Kully Nov 3, 2016
b2b8451
tests
Kully Nov 3, 2016
a156503
added create_animations so v2 is hit if frames is in fig
Kully Nov 4, 2016
d173cbb
create_animations to bad_create_animations
Kully Nov 8, 2016
d17a222
added function to spit out uids from column names
Kully Nov 14, 2016
49bdc97
made get_grid() to pull grid from cloud and added method in Grid clas…
Kully Nov 15, 2016
4fbdc99
cleaned up url parsing + moved json_to_grid functionality to __init__…
Kully Nov 16, 2016
dbb386f
fixed order upon creating Grid
Kully Nov 17, 2016
bd2132b
accounted for orders with jump discontinuities
Kully Nov 18, 2016
26c3714
moved all_columns list where appropriate - above code block that uses it
Kully Nov 19, 2016
0c7bb1c
...
Kully Nov 24, 2016
4acc146
commented out fid stuff
Kully Nov 24, 2016
c195e78
merged master into branch
Kully Nov 24, 2016
6f12ba8
delete print lines from debugging
Kully Nov 24, 2016
253228f
grid_id is set to grids created via get_grid
Kully Nov 24, 2016
405e71d
add 'Frames' to grid_objs_test
Kully Nov 24, 2016
c1727d8
removed commented out code for old get_uid code
Kully Nov 24, 2016
fca8fed
added animations function
Kully Nov 24, 2016
587dfa5
Chris' comments sans tests
Kully Nov 25, 2016
495b015
changed some terminology around
Kully Nov 26, 2016
dd20177
last of non-blocking
Kully Nov 26, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions plotly/graph_objs/graph_objs.py
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,22 @@ def __init__(self, *args, **kwargs):
_parent_key='data')
figure_class.__init__ = __init__

# TODO better integrate frames into Figure - #604
def __setitem__(self, key, value, **kwargs):
if key == 'frames':
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a comment about this one. Maybe tie it to a new issue to better-integrate frames into Figure? As always, pls put the issue numer (or link) in the TODO comment.

super(PlotlyDict, self).__setitem__(key, value)
else:
super(figure_class, self).__setitem__(key, value, **kwargs)
figure_class.__setitem__ = __setitem__

def _get_valid_attributes(self):
super(figure_class, self)._get_valid_attributes()
# TODO better integrate frames into Figure - #604
if 'frames' not in self._valid_attributes:
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto hear, link it to that same issue ^^

self._valid_attributes.add('frames')
return self._valid_attributes
figure_class._get_valid_attributes = _get_valid_attributes

def get_data(self, flatten=False):
"""
Returns the JSON for the plot with non-data elements stripped.
Expand Down
4 changes: 1 addition & 3 deletions plotly/graph_reference.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
'ErrorZ': {'object_name': 'error_z', 'base_type': dict},
'Figure': {'object_name': 'figure', 'base_type': dict},
'Font': {'object_name': 'font', 'base_type': dict},
'Frames': {'object_name': 'frames', 'base_type': dict},
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary? Did I accidentally leave this in or something ;P?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I added it in. I don't know if it's actually being used though.

'Heatmap': {'object_name': 'heatmap', 'base_type': dict},
'Histogram': {'object_name': 'histogram', 'base_type': dict},
'Histogram2d': {'object_name': 'histogram2d', 'base_type': dict},
Expand Down Expand Up @@ -87,7 +88,6 @@ def get_graph_reference():

graph_reference_url = '{}{}?sha1={}'.format(plotly_api_domain,
GRAPH_REFERENCE_PATH, sha1)

try:
response = requests.get(graph_reference_url,
timeout=GRAPH_REFERENCE_DOWNLOAD_TIMEOUT)
Expand Down Expand Up @@ -196,11 +196,9 @@ def get_valid_attributes(object_name, parent_object_names=()):
# These are for documentation and quick lookups. They're just strings.
valid_attributes = set()
for attributes_dict in attributes.values():

for key, val in attributes_dict.items():
if key not in GRAPH_REFERENCE['defs']['metaKeys']:
valid_attributes.add(key)

deprecated_attributes = attributes_dict.get('_deprecated', {})
for key, val in deprecated_attributes.items():
if key not in GRAPH_REFERENCE['defs']['metaKeys']:
Expand Down
105 changes: 93 additions & 12 deletions plotly/grid_objs/grid_objs.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,29 +118,96 @@ class Grid(MutableSequence):
py.plot([trace], filename='graph from grid')
```
"""
def __init__(self, iterable_of_columns):
def __init__(self, columns_or_json, grid_id=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, can we call this fid?

"""
Initialize a grid with an iterable of
`plotly.grid_objs.Column objects
Initialize a grid with an iterable of `plotly.grid_objs.Column`
objects or a json/dict describing a grid. See second usage example
below for the necessary structure of the dict.

Usage example:
:param (str|bool) grid_id: should not be accessible to users. Default
is 'None' but if a grid is retrieved via `py.get_grid()` then the
retrieved grid response will contain the grid_id which will be
necessary to set `self.id` and `self._columns.id` below.

Example from iterable of columns:
```
column_1 = Column([1, 2, 3], 'time')
column_2 = Column([4, 2, 5], 'voltage')
grid = Grid([column_1, column_2])
```
Example from json grid
```
grid_json = {
'cols': {
'time': {'data': [1, 2, 3], 'order': 0, 'uid': '4cd7fc'},
'voltage': {'data': [4, 2, 5], 'order': 1, 'uid': u'2744be'}
}
}
grid = Grid(grid_json)
Copy link
Contributor

Choose a reason for hiding this comment

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

💥 🎎 :)

```
"""

# TODO: verify that columns are actually columns
if isinstance(columns_or_json, dict):
# check that grid_id is entered
if grid_id is None:
raise exceptions.PlotlyError(
"If you are manually converting a raw json/dict grid "
"into a Grid instance, you must ensure that make "
Copy link
Contributor

Choose a reason for hiding this comment

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

🐐 ensure that make

"'grid_id' is set to your file ID. This looks like "
"'username:187'."
)
# TODO: verify that grid_id is a correct fid if a string
self.id = grid_id
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency, can we call this self.fid?


# check if 'cols' is a root key
if 'cols' not in columns_or_json:
raise exceptions.PlotlyError(
"'cols' must be a root key in your json grid."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how metadata is represented in a Grid() instance, as meta seems to only be used when uploading Grids. Right now, you can get_grid and if set to raw=True then you'll get the metadata as is.

I've loosened the warning message, saying that cols must be there, but meta is not being put into the Grid as I don't know what that would look like.

Is this okay for you?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

)

# check if 'data', 'order' and 'uid' are not in columns
grid_col_keys = ['data', 'order', 'uid']

for column_name in columns_or_json['cols']:
for key in grid_col_keys:
if key not in columns_or_json['cols'][column_name]:
raise exceptions.PlotlyError(
"Each column name of your dictionary must have "
"'data', 'order' and 'uid' as keys."
)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 cool.

# collect and sort all orders in case orders do not start
# at zero or there are jump discontinuities between them
all_orders = []
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@theengineear This method works. 👍

for column_name in columns_or_json['cols'].keys():
all_orders.append(columns_or_json['cols'][column_name]['order'])
all_orders.sort()

# put columns in order in a list
ordered_columns = []
for order in all_orders:
for column_name in columns_or_json['cols'].keys():
if columns_or_json['cols'][column_name]['order'] == order:
break

ordered_columns.append(Column(
columns_or_json['cols'][column_name]['data'],
column_name)
)
self._columns = ordered_columns

# fill in uids
Copy link
Contributor

Choose a reason for hiding this comment

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

🎎 uids --> column_ids.

for column in self:
column.id = self.id + ':' + columns_or_json['cols'][column.name]['uid']
Copy link
Contributor

Choose a reason for hiding this comment

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

🐄 we typically use format i think. non-blocking


column_names = [column.name for column in iterable_of_columns]
duplicate_name = utils.get_first_duplicate(column_names)
if duplicate_name:
err = exceptions.NON_UNIQUE_COLUMN_MESSAGE.format(duplicate_name)
raise exceptions.InputError(err)
else:
column_names = [column.name for column in columns_or_json]
duplicate_name = utils.get_first_duplicate(column_names)
if duplicate_name:
err = exceptions.NON_UNIQUE_COLUMN_MESSAGE.format(duplicate_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

raise exceptions.InputError(err)

self._columns = list(iterable_of_columns)
self.id = ''
self._columns = list(columns_or_json)
self.id = ''

def __repr__(self):
return self._columns.__repr__()
Expand Down Expand Up @@ -187,3 +254,17 @@ def get_column(self, column_name):
for column in self._columns:
if column.name == column_name:
return column

def get_fid_uid(self, column_name):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it called get_fid_uid? Maybe just get_uid? Also, are users supposed to use this? They shouldn't need to know about uids or srcs. If it's something users are playing with, I'd suggest get_column_reference or something (which should return the fid:uid pair (and could potentially handle 2d references).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, users are supposed to use this. And it does return the fid:uid pair

"""
Return uid of given column name in the grid by column name.

Returns an empty string if either the column name does not exist in
the grid or if the id of the specified column has the empty string id.
"""
uid = ''
for column in self._columns:
if column.name == column_name:
uid = column.id
break
return uid
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this raise if it can't find a uid? Is there a case where we would want to accept the empty string if a column uid cannot be found?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, it should. It's possible for the column_reference to be the empty string '' if a grid hasn't been uploaded yet, for instance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

grids are instantiated with ids so I don't think that's a risk here. For now, I think it's fine to have it just return the id of that column, whatever it happens to be.

43 changes: 36 additions & 7 deletions plotly/offline/offline.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,16 @@ def init_notebook_mode(connected=False):

def _plot_html(figure_or_data, config, validate, default_width,
default_height, global_requirejs):

figure = tools.return_figure_from_figure_or_data(figure_or_data, validate)
# force no validation if frames is in the call
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO this and link to some issue pls :)

# TODO - add validation for frames in call - #605
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed now 👍

if 'frames' in figure_or_data:
figure = tools.return_figure_from_figure_or_data(
figure_or_data, False
)
else:
figure = tools.return_figure_from_figure_or_data(
figure_or_data, validate
)

width = figure.get('layout', {}).get('width', default_width)
height = figure.get('layout', {}).get('height', default_height)
Expand All @@ -185,6 +193,8 @@ def _plot_html(figure_or_data, config, validate, default_width,
plotdivid = uuid.uuid4()
jdata = json.dumps(figure.get('data', []), cls=utils.PlotlyJSONEncoder)
jlayout = json.dumps(figure.get('layout', {}), cls=utils.PlotlyJSONEncoder)
if 'frames' in figure_or_data:
jframes = json.dumps(figure.get('frames', {}), cls=utils.PlotlyJSONEncoder)

configkeys = (
'editable',
Expand Down Expand Up @@ -225,11 +235,30 @@ def _plot_html(figure_or_data, config, validate, default_width,
link_text = link_text.replace('plot.ly', link_domain)
config['linkText'] = link_text

script = 'Plotly.newPlot("{id}", {data}, {layout}, {config})'.format(
id=plotdivid,
data=jdata,
layout=jlayout,
config=jconfig)
if 'frames' in figure_or_data:
script = '''
Plotly.plot(
'{id}',
{data},
{layout},
{config}
).then(function () {add_frames}).then(function(){animate})
Copy link

@rreusser rreusser Oct 19, 2016

Choose a reason for hiding this comment

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

See: plotly/plotly.js#1054

That PR has been merged and is on plotly.js/master (unreleased, but coming very soon?) and makes the syntax somewhat simpler. I'd elaborate here, but the PR description explains it fairly clearly. Calling addFrames separately will remain perfectly fine, but ideally will become unnecessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, fantastic. I think Chris mentioned that I'd have to do some rewriting when that gets added.

I'll leave it for now, but does that mean frames is available as a "valid attribute" for Python?

Choose a reason for hiding this comment

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

Yeah, should just be a simplification. Long story short, given data, layout, frames:

  • frames[i].data follows the same validation as data itself
  • frames[i].layout follows the same validation as layout

I'm not 100% sure what that means for validation, but on a strictly abstract level, it's conceptually simple.

Choose a reason for hiding this comment

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

Oh, sorry, unless you were referring to python arguments instead of data validation… That PR just changes the call signature. Not 100% sure the other concerns, but feel free to ask whatever questions you need and I'll do my best to get you what you need.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm referring to this py.iplot(fig) call where fig contains data, layout and frames. Right now, I'm still getting that:

'frames' is not allowed in 'figure'

Path To Error: ['frames']

Valid attributes for 'figure' at path [] under parents []:

    ['layout', 'data']

Choose a reason for hiding this comment

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

Ah, got it. Yeah, it should permit frames.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am still receiving the error. Anything I can do to fix? I sudo pip upgraded just now and it reported no new changes.

Copy link

@rreusser rreusser Oct 19, 2016

Choose a reason for hiding this comment

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

Hmm… not sure. It seems like that's a whitelisting issue on the plotly (not js) side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, so after playing follow-that-code for a while, I've traced it to the v2 plot schema Can we add frames to this?

'''.format(
id=plotdivid,
data=jdata,
layout=jlayout,
config=jconfig,
add_frames="{" + "return Plotly.addFrames('{id}',{frames}".format(
id=plotdivid, frames=jframes
) + ");}",
animate="{" + "Plotly.animate('{id}');".format(id=plotdivid) + "}"
)
else:
script = 'Plotly.newPlot("{id}", {data}, {layout}, {config})'.format(
id=plotdivid,
data=jdata,
layout=jlayout,
config=jconfig)

optional_line1 = ('require(["plotly"], function(Plotly) {{ '
if global_requirejs else '')
Expand Down
112 changes: 58 additions & 54 deletions plotly/offline/plotly.min.js

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion plotly/plotly/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,7 @@
grid_ops,
meta_ops,
file_ops,
get_config
get_config,
get_grid,
create_animations
)
82 changes: 81 additions & 1 deletion plotly/plotly/plotly.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from plotly.session import (sign_in, update_session_plot_options,
get_session_plot_options, get_session_credentials,
get_session_config)
from plotly.grid_objs import Grid, Column

__all__ = None

Expand Down Expand Up @@ -238,6 +239,7 @@ def plot(figure_or_data, validate=True, **plot_options):

plot_options = _plot_option_logic(plot_options)
res = _send_to_plotly(figure, **plot_options)

if res['error'] == '':
if plot_options['auto_open']:
_open_url(res['url'])
Expand Down Expand Up @@ -383,6 +385,7 @@ def get_figure(file_owner_or_url, file_id=None, raw=False):
raise exceptions.PlotlyError(
"The 'file_id' argument must be a non-negative number."
)

response = requests.get(plotly_rest_url + resource,
headers=headers,
verify=get_config()['plotly_ssl_verification'])
Expand Down Expand Up @@ -938,6 +941,7 @@ def upload(cls, grid, filename,

paths = filename.split('/')
parent_path = '/'.join(paths[0:-1])

filename = paths[-1]

if parent_path != '':
Expand All @@ -958,6 +962,7 @@ def upload(cls, grid, filename,
payload['parent_path'] = parent_path

upload_url = _api_v2.api_url('grids')

req = requests.post(upload_url, data=payload,
headers=_api_v2.headers(),
verify=get_config()['plotly_ssl_verification'])
Expand Down Expand Up @@ -1299,7 +1304,7 @@ def response_handler(cls, response):
"Plotly On-Premise server to request against this endpoint or "
"this endpoint may not be available yet.\nQuestions? "
"Visit community.plot.ly, contact your plotly administrator "
"or upgrade to a Pro account for 1-1 help: https://goo.gl/1YUVu9 "
"or upgrade to a Pro account for 1-1 help: https://goo.gl/1YUVu9 "
.format(url=get_config()['plotly_api_domain'])
)
else:
Expand Down Expand Up @@ -1449,6 +1454,81 @@ def _send_to_plotly(figure, **plot_options):
return r


def get_grid(grid_url, raw=False):
"""
Returns a JSON figure representation for the specified grid.
Copy link
Contributor

Choose a reason for hiding this comment

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

🎎 This feels a little misleading. Aren't we returning either JSON or a custom object. And, in the JSON case, it's a dict, right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct again! A oversight on my part.


:param (bool) raw: if False, will output a Grid instance of the JSON grid
being retrieved. If True, raw JSON will be returned.
"""
credentials = get_credentials()
validate_credentials(credentials)
username, api_key = credentials['username'], credentials['api_key']
headers = {'plotly-username': username,
'plotly-apikey': api_key,
'plotly-version': version.__version__,
'plotly-platform': 'python'}
upload_url = _api_v2.api_url('grids')
Copy link
Contributor

Choose a reason for hiding this comment

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

Did the _api_v2 thingy not have a get_headers method?


# extract path in grid url
url_path = six.moves.urllib.parse.urlparse(grid_url)[2][2:]
if url_path[-1] == '/':
url_path = url_path[0: -1]
url_path = url_path.replace('/', ':')

meta_get_url = upload_url + '/' + url_path
get_url = meta_get_url + '/content'

r = requests.get(get_url, headers=headers)
json_res = json.loads(r.text)

# make request to grab the grid id (fid)
r_meta = requests.get(meta_get_url, headers=headers)
json_res_meta = json.loads(r_meta.text)
retrieved_grid_id = json_res_meta['fid']

if raw is False:
return Grid(json_res, retrieved_grid_id)
else:
return json_res


def create_animations(figure, filename=None, sharing='public'):
Copy link
Member

Choose a reason for hiding this comment

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

We might want to create an i version of this that works in ipython notebooks like icreate_animations or else add it as like an output method.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

"""
Copy link
Member

Choose a reason for hiding this comment

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

We should tell the user that this is a beta endpoint that is subject to deprecation in the future.

Copy link
Member

Choose a reason for hiding this comment

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

And we should describe how this is similar and dissimilar to plotly.plotly.plot. Includes things like:

  • Folders aren't supported
  • Overwrite isn't supported
  • Animations via frames are supported
  • There might be some other things that I'm forgetting about.

Put description here.
Copy link
Member

Choose a reason for hiding this comment

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

⚡️


For parameter descriptions, see the doc string for `py.plot()`.
Copy link
Member

Choose a reason for hiding this comment

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

instead of py.plot, plotly.plotly.plot since py is just an alias.

`private` is not supported currently for param 'sharing'. Returns the
url for the plot if response is ok.
"""
Copy link
Member

Choose a reason for hiding this comment

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

We should include some basic examples of animations in here.

credentials = get_credentials()
validate_credentials(credentials)
username, api_key = credentials['username'], credentials['api_key']
auth = HTTPBasicAuth(str(username), str(api_key))
headers = {'Plotly-Client-Platform': 'python'}

json = {
'figure': figure,
'world_readable': 'true'
Copy link
Member

Choose a reason for hiding this comment

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

If we're JSON-ifying everything correctly in the request, then this can simply be True. You may need to add 'content-type': 'application/json' to the headers of the request

}

# set filename if specified
if filename:
json['filename'] = filename
Copy link
Member

@chriddyp chriddyp Nov 24, 2016

Choose a reason for hiding this comment

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

In py.plot, if the user specifies a / in the filename then we'll automatically make folders for them. That won't happen here. We might want to check for / in the filename and then issue a warning to the user that this beta function won't create folders automatically for them.


# set sharing
if sharing == 'public':
json['world_readable'] = 'true'
elif sharing == 'private':
json['world_readable'] = 'false'
Copy link
Member

Choose a reason for hiding this comment

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

we should add 'secret' in here. we should also throw an error if sharing isn't one of public, private, or secret


api_url = _api_v2.api_url('plots')
r = requests.post(api_url, auth=auth, headers=headers, json=json)
Copy link
Member

Choose a reason for hiding this comment

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

What happens when the request fails? It can fail for a number of reasons:

  • API key wasn't supplied
  • API key wasn't correct
  • User was offline
  • Request was throttled
  • Figure was malformed
  • Duplicate filename
  • Permission denied (can't save the file as private)
  • Server was down
  • And more

Errors can be detected by checking for non-200 level status codes or by calling r.raise_for_status(). Plotly returns the error in a standard format (something like {error: {message: 'error message'}} but I don't recall exactly) and we should parse the error message and then rethrow an error with that message for the user.

All of those errors should have a similar error response but you should try to hit those errors manually yourself to verify that the error handling works correctly and is helpful. Bonus points for including all of those types of errors inside actual tests so that we're sure that our error handling doesn't change without us knowing.


json_r = json.loads(r.text)
Copy link
Member

Choose a reason for hiding this comment

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

this can fail if the response wasn't JSON. we might want to do

try:
    parsed_response = r.json()
except:
    parsed_response = r.content

return json_r['file']['web_url']
Copy link
Member

Choose a reason for hiding this comment

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

I forget if this URL includes the title text at the end of the URL (e.g. https://plot.ly/~chris/15/my-title-of-my-graph) or not. We should make sure that we're returning the minimal version of the url like https://plot.ly/~chris/15 just like how py.plot does

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is minimal but will check



Copy link
Member

Choose a reason for hiding this comment

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

We should add tests for this function including the possible types of errors that can occur

def _open_url(url):
try:
from webbrowser import open as wbopen
Expand Down
2 changes: 1 addition & 1 deletion plotly/tests/test_core/test_graph_objs/test_graph_objs.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
OLD_CLASS_NAMES = ['AngularAxis', 'Annotation', 'Annotations', 'Area',
'Bar', 'Box', 'ColorBar', 'Contour', 'Contours',
'Data', 'ErrorX', 'ErrorY', 'ErrorZ', 'Figure',
'Font', 'Heatmap', 'Histogram', 'Histogram2d',
'Font', 'Frames', 'Heatmap', 'Histogram', 'Histogram2d',
'Histogram2dContour', 'Layout', 'Legend', 'Line',
'Margin', 'Marker', 'RadialAxis', 'Scatter',
'Scatter3d', 'Scene', 'Stream', 'Surface', 'Trace',
Expand Down