Animations/Frames in Python API #584

Kully · 2016-10-18T19:05:10Z

Relevant Issue: https://github.com/plotly/streambed/issues/8129

Kully · 2016-10-18T19:34:36Z

fyi, this is my PR for issue 8129

So far, iplot in offline can properly create a chart with animations in the notebook,
plot is next...

cc @theengineear @chriddyp @etpinard @rreusser

Kully · 2016-10-19T20:10:49Z

@theengineear plot and iplot both work online. I'm tackling online mode now and probably end up making that beta method...

Should I write tests now as I go for offline or just continue?

theengineear · 2016-10-19T20:12:33Z

We're in a bit of a time-crunch, and my bet is that writing tests for this will be difficult. I'd suggest moving on and getting it to work online next.

Nice job so far, btw!

//cc @chriddyp

rreusser · 2016-10-19T20:17:32Z

plotly/offline/offline.py

+            {data},
+            {layout},
+            {config}
+        ).then(function () {add_frames}).then(function(){animate})


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.

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?

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.

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.

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']

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

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

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

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?

etpinard · 2016-10-19T20:18:22Z

Should I write tests now as I go for offline or just continue?

@Kully if you feel like figuring out why #549 is still failing on CircleCI, be my guest.

Kully · 2016-10-19T20:23:17Z

@Kully if you feel like figuring out why #549 is still failing on CircleCI, be my guest.

Looks like you just rebuilt it eh?

Kully · 2016-10-20T18:46:03Z

Just threw in code from #586 without merging it formally

… started work on generating grid/plot in v2 in create_animations

Kully · 2016-10-28T15:55:53Z

plotly/plotly/plotly.py

@@ -1457,6 +1448,76 @@ def _send_to_plotly(figure, **plot_options):
    return r


+def create_animations(fig, kwargs, payload):


@theengineear @rreusser

Would appreciate some guidance...

I wrote something that can pretty reliably break apart a simple figure (2D Scatterplots for example) and make a grid then plot, and then a request to the V2 REST API. I't's working and I can view my stuff on the cloud.

I am getting an error when trying to pass frames into the figure for the plot generation: I just get some gargled HTML. What is the next step for me getting frames to pass?

A few more things I need to tackle:
-make sure I include things like sharing in the create_animations call.
-eventually, once frames is being allowed to exist in DATA, validate items in frames the same way I am here

Any thoughts/ideas are appreciated.

🚨 💀 😨 I think you should not do figure splitting (i.e., exchanging raw data for referenced data). Please take my word on it. It's not worth it. It's extremely complicated (Please take my word on it).

^^ Seriously, take my word on it.

Users should be able to do something like:

grid = Grid() grid.add_column('time_data', [1, 2, 3, 4]) # something like this... grid.add_column('voltage', [120, 119, 122, 121]) # something like this... # somehow, someway, add data to this grid and make it easy for users to pull out the reference. # try looking in https://github.com/plotly/plotly.py/blob/master/plotly/grid_objs/grid_objs.py figure.data[0].x = grid.time_data figure.frames[0]['data'][0] = grid.time_data # Not sure if this is right, just for example create_animations(figure, ...)

AFAIK, you don't need to worry too much about checking for raw data in the figure. The api will catch you and the request will just fail.

Let's fully shift the responsibility to the user. If we get into the game of guessing, we'll just end up guessing wrong and creating bugs.

Kully · 2016-11-04T18:53:55Z

@theengineear @rreusser

So finally I have a create_animations which deals with a fig with frames in its body. Plotting works okay, just two things:

I don't think (know) if animations are live on plotly. @chriddyp Do you know if they work now?
frames is not showing up here so I don't know if they are completely passing through.

Additionally a GRID as well as a PLOT is made in the user's account each time frames is included, as v2 is being used. Also just realized that all GRIDS are hard-coded as world_readable. I can change this later but just noting it here.

Andrew, do you mind looking at my function and giving some PEP-8/style/structure/all-that-fun-stuff comments if you have any? :)

theengineear · 2016-11-04T23:14:39Z

plotly/graph_objs/graph_objs.py

@@ -836,6 +836,20 @@ def __init__(self, *args, **kwargs):
                                                  _parent_key='data')
    figure_class.__init__ = __init__

+    def __setitem__(self, key, value, **kwargs):
+        if key == 'frames':


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.

theengineear · 2016-11-04T23:15:07Z

plotly/graph_objs/graph_objs.py

+
+    def _get_valid_attributes(self):
+        super(figure_class, self)._get_valid_attributes()
+        if 'frames' not in self._valid_attributes:


Ditto hear, link it to that same issue ^^

theengineear · 2016-11-04T23:16:18Z

plotly/graph_reference.py

@@ -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},


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

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

theengineear · 2016-11-04T23:16:57Z

plotly/offline/offline.py

@@ -162,8 +162,15 @@ 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


TODO this and link to some issue pls :)

theengineear · 2016-11-04T23:19:24Z

plotly/plotly/plotly.py

@@ -1400,9 +1400,6 @@ def add_share_key_to_url(plot_url, attempt=0):


 def _send_to_plotly(figure, **plot_options):
-    """
-
-    """


This has been an empty comment for apparently one year. Should I attempt writing something in it and get it corrected?

oop, sorry. Weird. I just saw the change and questioned it. sure 🔪 it :)

Write one or 'cut it'?

cut it is fine :)

theengineear · 2016-11-04T23:24:40Z

plotly/plotly/plotly.py

+
+    # add layout if not in fig
+    if 'layout' not in fig:
+        fig['layout'] = {}


Unnecessary comment and this can be a clean one-liner as you've done before.

theengineear · 2016-11-04T23:25:18Z

plotly/plotly/plotly.py

+        fig['layout'] = {}
+
+    # make a copy of fig
+    fig_with_uids = copy.deepcopy(fig)


Another unnecessary comment. Just do this and the above layout thing in two lines.

theengineear · 2016-11-04T23:25:54Z

plotly/plotly/plotly.py

+    # make a copy of fig
+    fig_with_uids = copy.deepcopy(fig)
+
+    # make grid


This, on the other hand, is a nice little comment, imo.

theengineear · 2016-11-04T23:31:27Z

plotly/plotly/plotly.py

+                cols_dict["{name}, {x_or_y}".format(name=trace['name'],
+                                                    x_or_y=var)] = {
+                    "data": list(trace[var]), "order": counter
+                }


🐄 if it's going to take up 4 lines anyhow, I'd suggest something like?

key = '{}, {}'.format(trace['name'], var) data = list(trace[var]) cols[key] = {'data': data, 'order': counter}

theengineear · 2016-11-04T23:51:02Z

plotly/plotly/plotly.py

@@ -1457,6 +1448,76 @@ def _send_to_plotly(figure, **plot_options):
    return r


+def create_animations(fig, kwargs, payload):


🚨 💀 😨 I think you should not do figure splitting (i.e., exchanging raw data for referenced data). Please take my word on it. It's not worth it. It's extremely complicated (Please take my word on it).

^^ Seriously, take my word on it.

Users should be able to do something like:

grid = Grid() grid.add_column('time_data', [1, 2, 3, 4]) # something like this... grid.add_column('voltage', [120, 119, 122, 121]) # something like this... # somehow, someway, add data to this grid and make it easy for users to pull out the reference. # try looking in https://github.com/plotly/plotly.py/blob/master/plotly/grid_objs/grid_objs.py figure.data[0].x = grid.time_data figure.frames[0]['data'][0] = grid.time_data # Not sure if this is right, just for example create_animations(figure, ...)

AFAIK, you don't need to worry too much about checking for raw data in the figure. The api will catch you and the request will just fail.

Let's fully shift the responsibility to the user. If we get into the game of guessing, we'll just end up guessing wrong and creating bugs.

Kully · 2016-11-09T16:46:18Z

Here's the gameplan. I'll remake the create_animations function to be less 💀 . 😜

According to @etpinard, animations aren't supported in shareplot, embed yet.

@theengineear, would you know anything about the .json endpoint? This seems like a later-down-the-line task but right now, frames is not throwing an error (because of your workaround) but it may be getting lost somewhere in the streambed code.

So a recap:

Rewrite create_animations to put more responsibility on the user
Wait for frames to be supported in embed, shareplot
ensure that .json is being updated properly here

Please advise different approaches if you think of any

theengineear · 2016-11-14T22:11:40Z

plotly/offline/offline.py

@@ -163,6 +163,7 @@ def init_notebook_mode(connected=False):
 def _plot_html(figure_or_data, config, validate, default_width,
               default_height, global_requirejs):
    # force no validation if frames is in the call
+    # TODO - add validation for frames in call - #


Thanks for linking issues ❤️! Looks like this one may have been missed though.

LOL, whoops! That's #605

theengineear · 2016-11-14T22:14:19Z

plotly/plotly/plotly.py

@@ -1576,6 +1575,39 @@ def bad_create_animations(fig, kwargs, payload):
    return r_dict


+def get_uid_by_col_name(grid_url, col_name):


The grid_url arg should be a little more flexible. Look at https://github.com/plotly/plotly.py/blob/master/plotly/plotly/plotly.py#L318. I'm OK with this as is. But I could see it being frustrating to be able to reference files one way in one place and another way in another place.

Also. I sort of imagined that this would have been part of our Grid class? More like:

1. Get `Grid` from `grid_url` (symmetric to `get_figure`), this is a network operation 2. Get `uid` from `Grid` (which is a simple, local operation)

…s to retrieve uid from column name

Kully · 2016-11-15T21:18:58Z

plotly/offline/offline.py

@@ -163,7 +163,7 @@ def init_notebook_mode(connected=False):
 def _plot_html(figure_or_data, config, validate, default_width,
               default_height, global_requirejs):
    # force no validation if frames is in the call
-    # TODO - add validation for frames in call - #
+    # TODO - add validation for frames in call - #605


Fixed now 👍

chriddyp · 2016-11-24T16:18:06Z

@mdtusz - If validation is the only thing that's blocking, I'm think that it has been implemented in this branch already. You could work off of this branch in your environment with something like:

$ pip install git+https://github.com/plotly/plotly.py.git@animations-for-iqt2

Kully · 2016-11-24T16:21:48Z

I'm think that it has been implemented in this branch already

And it has!

Kully · 2016-11-24T20:24:09Z

@chriddyp @jackparmer @mdtusz @theengineear

Okay, everything should be ready. Just need a review:

Firstly, this is how a user can create a grid, retrieve the fid:uid combination and fill in a payload variable to make a POST to create the plot:

import plotly.plotly as py
from plotly.grid_objs import grid_objs

column_1 = grid_objs.Column([3, 1, 4], 'apple')
column_2 = grid_objs.Column([1, 5, 9], 'pies')
column_3 = grid_objs.Column([1, 1, 1], 'for')
column_4 = grid_objs.Column([6, 6, 6], 'sale')
grid = grid_objs.Grid([column_1, column_2, column_3, column_4])

py.grid_ops.upload(grid, 'bake_sale', auto_open=False)

then grab a the fid:uid by column name using grid.get_fid_uid('apple') for instance. Last step is to create the plot:

import json
import requests
from requests.auth import HTTPBasicAuth
 
auth = HTTPBasicAuth('AdamKulidjian', 'nlpvm9aack')
headers = {'Plotly-Client-Platform': 'python'}
 
payload = {
    'figure': {
        'data': [{'xsrc': grid.get_fid_uid('apple'), 'ysrc': grid.get_fid_uid('pies')}],
        'layout': {'title': 'Apples are Cool.'},
        'frames': [ {'data': [{'xsrc': grid.get_fid_uid('for'), 'ysrc': grid.get_fid_uid('sale')}],
                     'layout': {'title': 'They Are Nice'}}]
    },
    'world_readable': 'true'
}

r = requests.post('https://api.plot.ly/v2/plots', auth=auth, headers=headers, json=payload)

You can find the result at the .json endpoint here

A few notes about the PR:

It doesn't seem mandatory at this point, but I had written a py.get_grid() function which allows a user to grab a preexisting grid from the cloud in either JSON form or Grid (cls) form. It's handy if the grids have already been created and we need to plot from them.
Vis a Vis the functionality in 1), users actually have the ability to input a grid_id as a param into Grid(). Instead of setting the grid and their column id's to empty strings, if a string is entered then it will set that to the current id. Is there a way to make this variable unreachable by the user?

chriddyp · 2016-11-24T21:04:25Z

Re

Last step is to create the plot:
```
import json
import requests
from requests.auth import HTTPBasicAuth

auth = HTTPBasicAuth('AdamKulidjian', 'nlpvm9aack')
headers = {'Plotly-Client-Platform': 'python'}
 
payload = {
    'figure': {
        'data': [{'xsrc': grid.get_fid_uid('apple'), 'ysrc': grid.get_fid_uid('pies')}],
        'layout': {'title': 'Apples are Cool.'},
        'frames': [ {'data': [{'xsrc': grid.get_fid_uid('for'), 'ysrc': grid.get_fid_uid('sale')}],
                     'layout': {'title': 'They Are Nice'}}]
    },
    'world_readable': 'true'
}

r = requests.post('https://api.plot.ly/v2/plots', auth=auth, headers=headers, json=payload)
```

Does this PR also include a beta function that wraps this up nicely? From the previous discussion, I proposed something like:

Create an animation on the server: py.create_animation(figure, filename, sharing)

Kully · 2016-11-24T21:18:05Z

Does this PR also include a beta function that wraps this up nicely?

Nope, but I'll write one up now!

chriddyp · 2016-11-24T22:12:40Z

plotly/plotly/plotly.py

+
+    json = {
+        'figure': figure,
+        'world_readable': 'true'


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

chriddyp · 2016-11-24T22:14:58Z

plotly/plotly/plotly.py

+    api_url = _api_v2.api_url('plots')
+    r = requests.post(api_url, auth=auth, headers=headers, json=json)
+
+    json_r = json.loads(r.text)


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

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

chriddyp · 2016-11-24T22:20:09Z

plotly/plotly/plotly.py

+        json['world_readable'] = 'false'
+
+    api_url = _api_v2.api_url('plots')
+    r = requests.post(api_url, auth=auth, headers=headers, json=json)


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.

chriddyp · 2016-11-24T22:21:06Z

plotly/plotly/plotly.py

+
+    # set filename if specified
+    if filename:
+        json['filename'] = filename


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.

chriddyp · 2016-11-24T22:21:45Z

plotly/plotly/plotly.py

+    if sharing == 'public':
+        json['world_readable'] = 'true'
+    elif sharing == 'private':
+        json['world_readable'] = 'false'


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

chriddyp · 2016-11-24T22:22:17Z

plotly/plotly/plotly.py

@@ -1493,6 +1493,42 @@ def get_grid(grid_url, raw=False):
        return json_res


+def create_animations(figure, filename=None, sharing='public'):
+    """
+    Put description here.


chriddyp · 2016-11-24T22:22:43Z

plotly/plotly/plotly.py

+    """
+    Put description here.
+
+    For parameter descriptions, see the doc string for `py.plot()`.


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

chriddyp · 2016-11-24T22:23:04Z

plotly/plotly/plotly.py

+    For parameter descriptions, see the doc string for `py.plot()`.
+    `private` is not supported currently for param 'sharing'. Returns the
+    url for the plot if response is ok.
+    """


We should include some basic examples of animations in here.

chriddyp · 2016-11-24T22:23:29Z

plotly/plotly/plotly.py

@@ -1493,6 +1493,42 @@ def get_grid(grid_url, raw=False):
        return json_res


+def create_animations(figure, filename=None, sharing='public'):
+    """


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

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.

chriddyp · 2016-11-24T22:25:19Z

plotly/plotly/plotly.py

@@ -1493,6 +1493,42 @@ def get_grid(grid_url, raw=False):
        return json_res


+def create_animations(figure, filename=None, sharing='public'):


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.

chriddyp · 2016-11-24T22:27:27Z

plotly/plotly/plotly.py

+    r = requests.post(api_url, auth=auth, headers=headers, json=json)
+
+    json_r = json.loads(r.text)
+    return json_r['file']['web_url']


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

I think it is minimal but will check

chriddyp · 2016-11-24T22:27:49Z

plotly/plotly/plotly.py

+    json_r = json.loads(r.text)
+    return json_r['file']['web_url']
+
+


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

theengineear

Couple requests (nothing too major). One thing that is still confusing me though:

How is a user supposed to populate *src fields with something like adam:10:aaaaaa?

theengineear · 2016-11-26T00:10:36Z

plotly/grid_objs/grid_objs.py

+                    "'username:187'."
+                )
+            # TODO: verify that grid_id is a correct fid if a string
+            self.id = grid_id


For consistency, can we call this self.fid?

theengineear · 2016-11-26T00:11:04Z

plotly/grid_objs/grid_objs.py

@@ -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):


For consistency, can we call this fid?

theengineear · 2016-11-26T00:13:59Z

plotly/grid_objs/grid_objs.py

@@ -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):


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).

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

theengineear · 2016-11-26T00:15:34Z

plotly/grid_objs/grid_objs.py

+            if column.name == column_name:
+                uid = column.id
+                break
+        return uid


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?

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.

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.

theengineear · 2016-11-26T00:18:40Z

plotly/plotly/plotly.py

@@ -1449,6 +1454,194 @@ 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.


🎎 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.

Correct again! A oversight on my part.

theengineear · 2016-11-26T00:19:46Z

plotly/plotly/plotly.py

+               'plotly-apikey': api_key,
+               'plotly-version': version.__version__,
+               'plotly-platform': 'python'}
+    upload_url = _api_v2.api_url('grids')


Did the _api_v2 thingy not have a get_headers method?

theengineear · 2016-11-26T00:21:52Z

plotly/plotly/plotly.py

+    to `plotly.plotly.plot`, folder-creation and overwriting are not supported
+    but creating a plot with or without animations via frames is supported.
+
+    :param (str) filename: if set to 'None', an automatically generated plot


🐐 automatically-generated I think.

theengineear · 2016-11-26T00:22:58Z

plotly/plotly/plotly.py

+    username, api_key = credentials['username'], credentials['api_key']
+    auth = HTTPBasicAuth(str(username), str(api_key))
+    headers = {'Plotly-Client-Platform': 'python',
+               'content-type': 'application/json'}


Again, I think we should have a get_headers function or something to reuse code a bit here.

There's this _api_v2.headers() which returns for me

{'authorization': u'Basic QWRhbUt1bGlkamlhbjpubHB2bTlhYWNr', 'plotly-client-platform': 'python 1.12.10'}

The thing is, usually I'm adding more things to headers so it wouldn't exactly make it more DRY ya know? @theengineear

But you could improve the _api_v2.headers() method to allow for more things! 🐫! Centralizing functionality like this is almost always a good thing!

theengineear · 2016-11-26T00:24:00Z

plotly/plotly/plotly.py

+
+    json = {
+        'figure': figure,
+        'world_readable': True


Hm. @chriddyp not sure if you weighed in on this, but we typically default to world_readable: false, right?

I think that py.plot actually defaults to True. Moving forward, defaulting to False would probably be the right move.

We should double check the error message that gets returned when you hit this private file limit. We may want to modify the error message before we return it so that we can include helpful language like "Set "sharing" to "public" if you want to publish this graph with public access" or something (i.e. include the keyword "sharing" in the error message so that users know how to continue.)

what is the private file limit? Is this for a particular type of subscription?

It's 1 for Community users (similar to GitHub) - See https://plot.ly/products/cloud/

I don't think it makes sense to make set sharing to private as a default if the Community users get only 1. I think that may be confusing and more work if they have to set it to public each time they make a plot.

An error message for hitting the private limit makes sense

theengineear

OK @Kully! I'm officially making these things non-blocking. I'd love it if you could make some changes based on my comments, but I'm ready to give this a 💃!

Nice job getting through this one! 💥 💣 ⚡️ . Let me know if you need a final round of review though, I'm always happy to do so.

theengineear · 2016-11-26T04:32:12Z

plotly/grid_objs/grid_objs.py

+            if fid is None:
+                raise exceptions.PlotlyError(
+                    "If you are manually converting a raw json/dict grid "
+                    "into a Grid instance, you must ensure that make "


🐐 ensure that make

theengineear · 2016-11-26T04:32:56Z

plotly/grid_objs/grid_objs.py

+                    "'fid' is set to your file ID. This looks like "
+                    "'username:187'."
+                )
+            # TODO: verify that fid is a correct fid if a string


You mean validate it? It'd be a whole extra api call to actually check. I think you don't need to worry about it.

theengineear · 2016-11-26T04:33:35Z

plotly/grid_objs/grid_objs.py

+                    "'username:187'."
+                )
+            # TODO: verify that fid is a correct fid if a string
+            self.id = fid


🐄 non-blocking, but I'd go with self.fid here.

Actually, if you call grid.id you'll get the fid back. That was in place before I started working on this PR. so id === fid for the grid.

theengineear · 2016-11-26T04:35:00Z

plotly/grid_objs/grid_objs.py

+                )
+            self._columns = ordered_columns
+
+            # fill in uids


🎎 uids --> column_ids.

theengineear · 2016-11-26T04:35:25Z

plotly/grid_objs/grid_objs.py

+
+            # fill in uids
+            for column in self:
+                column.id = self.id + ':' + columns_or_json['cols'][column.name]['uid']


🐄 we typically use format i think. non-blocking

theengineear · 2016-11-26T04:39:50Z

plotly/grid_objs/grid_objs.py

+            raise exceptions.PlotlyError(
+                "Whoops, that column name doesn't match any of the column "
+                "names in your grid. You must pick from {cols}".format(col_names)
+            )


theengineear · 2016-11-26T04:42:13Z

plotly/plotly/plotly.py

+    username, api_key = credentials['username'], credentials['api_key']
+    auth = HTTPBasicAuth(str(username), str(api_key))
+    headers = {'Plotly-Client-Platform': 'python',
+               'content-type': 'application/json'}


But you could improve the _api_v2.headers() method to allow for more things! 🐫! Centralizing functionality like this is almost always a good thing!

theengineear · 2016-11-26T04:43:29Z

plotly/plotly/plotly.py

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


Any way to narrow this exception clause? It's too broad. If you just copied this code, I'm OK with leaving it as-is. Just sets off 🚨 when I see it.

theengineear · 2016-11-26T04:44:38Z

plotly/plotly/plotly.py

+        parsed_response = r.content
+
+    if 'error' in r and r['error'] != '':
+        raise exceptions.PlotlyError(r['message'])


huh? Wouldn't this want to print out r['error'] If that's not the case can you add a quick code comment? This is a little confusing to me.

theengineear · 2016-11-26T04:45:54Z

plotly/plotly/plotly.py

+    This function is based off `plotly.plotly.iplot`. See `plotly.plotly.
+    create_animations` Doc String for param descriptions.
+    """
+    # TODO - create a wrapper for iplot and icreate_animations


Still need doing?

iplot in offline works with frames

cb4552c

updated js version in plotly offline

b4c3953

rreusser reviewed Oct 19, 2016

View reviewed changes

added Andrew's #586 PR

54c1386

added some print statements to check progresses

d19c6ca

Kully mentioned this pull request Oct 25, 2016

Patch Figure to allow frames. #586

Closed

Kully self-assigned this Oct 26, 2016

__init__.py now accepts create_animations which lives in plotly.py --…

efa390e

… started work on generating grid/plot in v2 in create_animations

Kully commented Oct 28, 2016

View reviewed changes

Kully added 4 commits November 3, 2016 17:28

rewrote create_animations to take frames - still beta

eb57218

merged master offline.py to local - changed call signature in one func

5482b53

tests

b2b8451

added create_animations so v2 is hit if frames is in fig

a156503

theengineear suggested changes Nov 4, 2016

View reviewed changes

create_animations to bad_create_animations

d173cbb

This was referenced Nov 14, 2016

Better Integrate frames into Figure #604

Closed

Add Validation check to frames in offline mode #605

Closed

added function to spit out uids from column names

d17a222

theengineear reviewed Nov 14, 2016

View reviewed changes

made get_grid() to pull grid from cloud and added method in Grid clas…

49bdc97

…s to retrieve uid from column name

Kully commented Nov 15, 2016

View reviewed changes

Kully added 5 commits November 24, 2016 11:30

commented out fid stuff

4acc146

merged master into branch

c195e78

delete print lines from debugging

6f12ba8

grid_id is set to grids created via get_grid

253228f

add 'Frames' to grid_objs_test

405e71d

Kully added 2 commits November 24, 2016 16:19

removed commented out code for old get_uid code

c1727d8

added animations function

fca8fed

chriddyp reviewed Nov 24, 2016

View reviewed changes

Chris' comments sans tests

587dfa5

theengineear suggested changes Nov 26, 2016

View reviewed changes

changed some terminology around

495b015

theengineear approved these changes Nov 26, 2016

View reviewed changes

last of non-blocking

dd20177

Kully merged commit febe6f4 into master Nov 26, 2016

Kully deleted the animations-for-iqt2 branch November 26, 2016 22:23

Kully mentioned this pull request Dec 6, 2016

Better error messages for (i)create_animations #628

Merged

8 tasks

		@@ -1457,6 +1448,76 @@ def _send_to_plotly(figure, **plot_options):
		return r


		def create_animations(fig, kwargs, payload):

		@@ -1576,6 +1575,39 @@ def bad_create_animations(fig, kwargs, payload):
		return r_dict


		def get_uid_by_col_name(grid_url, col_name):

		@@ -1493,6 +1493,42 @@ def get_grid(grid_url, raw=False):
		return json_res


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

Animations/Frames in Python API #584

Animations/Frames in Python API #584

Conversation

Kully commented Oct 18, 2016

Kully commented Oct 18, 2016 • edited Loading

Kully commented Oct 19, 2016

theengineear commented Oct 19, 2016

rreusser Oct 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rreusser Oct 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etpinard commented Oct 19, 2016

Kully commented Oct 19, 2016

Kully commented Oct 20, 2016

Kully Oct 28, 2016 • edited Loading

Choose a reason for hiding this comment

theengineear Nov 4, 2016 • edited by Kully Loading

Choose a reason for hiding this comment

Kully commented Nov 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theengineear Nov 4, 2016 • edited by Kully Loading

Choose a reason for hiding this comment

Kully commented Nov 9, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriddyp commented Nov 24, 2016

Kully commented Nov 24, 2016

Kully commented Nov 24, 2016 • edited Loading

chriddyp commented Nov 24, 2016

Kully commented Nov 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriddyp Nov 24, 2016 • edited by Kully Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theengineear left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kully commented Oct 18, 2016 •

edited

Loading

rreusser Oct 19, 2016 •

edited

Loading

rreusser Oct 19, 2016 •

edited

Loading

Kully Oct 28, 2016 •

edited

Loading

theengineear Nov 4, 2016 •

edited by Kully

Loading

theengineear Nov 4, 2016 •

edited by Kully

Loading

Kully commented Nov 24, 2016 •

edited

Loading

chriddyp Nov 24, 2016 •

edited by Kully

Loading