Repository: plotly/plotly.py

Issue:
Better Integrate `frames` into `Figure`
Referring to fix in https://github.com/plotly/plotly.py/pull/584 -> https://github.com/plotly/plotly.py/blob/a156503b275763a3ec7738f5f251b93597824ed0/plotly/graph_objs/graph_objs.py#L839

@theengineear 

Discussion:
theengineear: @Kully and I hacked in frames support when we were in a rush. This improves how `frames` is integrated into the `Figure` object. Since `frames` is basically considered an array of `Figure` objects.

Still working on this one, since I think saying that an entry in a `Frames` array is a `Figure` is misleading, imo...
theengineear: Can't add `Frame` since it conflicts with `animation.frame` and the name doesn't matter since we're not exposing it :). I chose `frame_entry`, which does *not* become a public class because there's no need to patch the object. Note how it ends up as `dict( .. )` in the `to_string()` call.
theengineear: @etpinard , OK, this would work well from the Python side of things for the `frames` object:

```
    "frames": {
        "items": {
            "frames_entry": {
                "baseframe": {
                    "description": "The name of the frame into which this frame's properties are merged before applying. This is used to unify properties and avoid needing to specify the same values for the same properties in multiple frames.",
                    "role": "info",
                    "valType": "string"
                },
                "data": {
                    "description": "A list of traces this frame modifies. The format is identical to the normal trace definition.",
                    "role": "object",
                    "valType": "any"
                },
                "group": {
                    "description": "An identifier that specifies the group to which the frame belongs, used by animate to select a subset of frames.",
                    "role": "info",
                    "valType": "string"
                },
                "layout": {
                    "role": "object",
                    "description": "Layout properties which this frame modifies. The format is identical to the normal layout definition.",
                    "valType": "any"
                },
                "name": {
                    "description": "A label by which to identify the frame",
                    "role": "info",
                    "valType": "string"
                },
                "traces": {
                    "description": "A list of trace indices that identify the respective traces in the data attribute",
                    "role": "info",
                    "valType": "info_array"
                },
                "role": "object"
            }
        },
        "role": "object"
    },
```

The name `frames_entry` is up for discussion but it would greatly help me if it's unique in the plot-schema, fwiw... i.e., not `frame`. This is because it's tricky to map `data` and `layout` attrs properly inside `frames_entry` objects.

Note that `FrameEntry` will *not* be a name that is exposed to users in the Python API, it's merely helpful for mapping attrs.
theengineear: OK, I "updated" the default schema with what I'd *like* to be in there. Obviously not ready to merge since this isn't a real reflection of the plot-schema, I just figured It'd be helpful.
theengineear: Oh right, we have a test on the schema anyhow so I *can't* merge it 😊 
etpinard: @theengineear done (I think) in https://github.com/plotly/plotly.js/pull/1292

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b8d15791e..fe8ea889e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Updated
 - `plotly.plotly.create_animations` and `plotly.plotly.icreate_animations` now return appropriate error messages if the response is not successful.
+- `frames` are now integrated into GRAPH_REFERENCE and figure validation.
 
 ### Changed
 - The plot-schema from `https://api.plot.ly/plot-schema` is no longer updated on import.
diff --git a/plotly/graph_objs/graph_objs.py b/plotly/graph_objs/graph_objs.py
index 10e6221f0..c21f41aa3 100644
--- a/plotly/graph_objs/graph_objs.py
+++ b/plotly/graph_objs/graph_objs.py
@@ -787,7 +787,7 @@ class GraphObjectFactory(object):
 
         # We patch Figure and Data, so they actually require the subclass.
         class_name = graph_reference.OBJECT_NAME_TO_CLASS_NAME.get(object_name)
-        if class_name in ['Figure', 'Data']:
+        if class_name in ['Figure', 'Data', 'Frames']:
             return globals()[class_name](*args, **kwargs)
         else:
             kwargs['_name'] = object_name
@@ -1097,7 +1097,7 @@ class Figure(PlotlyDict):
     """
     Valid attributes for 'figure' at path [] under parents ():
     
-        ['data', 'layout']
+        ['data', 'frames', 'layout']
     
     Run `<figure-object>.help('attribute')` on any of the above.
     '<figure-object>' is the object at []
@@ -1108,22 +1108,7 @@ class Figure(PlotlyDict):
     def __init__(self, *args, **kwargs):
         super(Figure, self).__init__(*args, **kwargs)
         if 'data' not in self:
-            self.data = GraphObjectFactory.create('data', _parent=self,
-                                                  _parent_key='data')
-
-    # TODO better integrate frames into Figure - #604
-    def __setitem__(self, key, value, **kwargs):
-        if key == 'frames':
-            super(PlotlyDict, self).__setitem__(key, value)
-        else:
-            super(Figure, self).__setitem__(key, value, **kwargs)
-
-    def _get_valid_attributes(self):
-        super(Figure, self)._get_valid_attributes()
-        # TODO better integrate frames into Figure - #604
-        if 'frames' not in self._valid_attributes:
-            self._valid_attributes.add('frames')
-        return self._valid_attributes
+            self.data = Data(_parent=self, _parent_key='data')
 
     def get_data(self, flatten=False):
         """
@@ -1241,8 +1226,45 @@ class Font(PlotlyDict):
     _name = 'font'
 
 
-class Frames(dict):
-    pass
+class Frames(PlotlyList):
+    """
+    Valid items for 'frames' at path [] under parents ():
+        ['dict']
+
+    """
+    _name = 'frames'
+
+    def _value_to_graph_object(self, index, value, _raise=True):
+        if isinstance(value, six.string_types):
+            return value
+        return super(Frames, self)._value_to_graph_object(index, value,
+                                                          _raise=_raise)
+
+    def to_string(self, level=0, indent=4, eol='\n',
+                  pretty=True, max_chars=80):
+        """Get formatted string by calling `to_string` on children items."""
+        if not len(self):
+            return "{name}()".format(name=self._get_class_name())
+        string = "{name}([{eol}{indent}".format(
+            name=self._get_class_name(),
+            eol=eol,
+            indent=' ' * indent * (level + 1))
+        for index, entry in enumerate(self):
+            if isinstance(entry, six.string_types):
+                string += repr(entry)
+            else:
+                string += entry.to_string(level=level+1,
+                                          indent=indent,
+                                          eol=eol,
+                                          pretty=pretty,
+                                          max_chars=max_chars)
+            if index < len(self) - 1:
+                string += ",{eol}{indent}".format(
+                    eol=eol,
+                    indent=' ' * indent * (level + 1))
+        string += (
+            "{eol}{indent}])").format(eol=eol, indent=' ' * indent * level)
+        return string
 
 
 class Heatmap(PlotlyDict):
diff --git a/plotly/graph_objs/graph_objs_tools.py b/plotly/graph_objs/graph_objs_tools.py
index 950adab94..fdd4cf713 100644
--- a/plotly/graph_objs/graph_objs_tools.py
+++ b/plotly/graph_objs/graph_objs_tools.py
@@ -35,8 +35,14 @@ def get_help(object_name, path=(), parent_object_names=(), attribute=None):
 def _list_help(object_name, path=(), parent_object_names=()):
     """See get_help()."""
     items = graph_reference.ARRAYS[object_name]['items']
-    items_classes = [graph_reference.string_to_class_name(item)
-                     for item in items]
+    items_classes = set()
+    for item in items:
+        if item in graph_reference.OBJECT_NAME_TO_CLASS_NAME:
+            items_classes.add(graph_reference.string_to_class_name(item))
+        else:
+            # There are no lists objects which can contain list entries.
+            items_classes.add('dict')
+    items_classes = list(items_classes)
     items_classes.sort()
     lines = textwrap.wrap(repr(items_classes), width=LINE_SIZE-TAB_SIZE-1)
 
diff --git a/plotly/graph_reference.py b/plotly/graph_reference.py
index 007824f0f..1caade10c 100644
--- a/plotly/graph_reference.py
+++ b/plotly/graph_reference.py
@@ -33,7 +33,7 @@ _BACKWARDS_COMPAT_CLASS_NAMES = {
     '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},
+    'Frames': {'object_name': 'frames', 'base_type': list},
     'Heatmap': {'object_name': 'heatmap', 'base_type': dict},
     'Histogram': {'object_name': 'histogram', 'base_type': dict},
     'Histogram2d': {'object_name': 'histogram2d', 'base_type': dict},
@@ -68,9 +68,62 @@ def get_graph_reference():
     """
     path = os.path.join('package_data', 'default-schema.json')
     s = resource_string('plotly', path).decode('utf-8')
-    graph_reference = _json.loads(s)
+    graph_reference = utils.decode_unicode(_json.loads(s))
+
+    # TODO: Patch in frames info until it hits streambed. See #659
+    graph_reference['frames'] = {
+          "items": {
+              "frames_entry": {
+                  "baseframe": {
+                      "description": "The name of the frame into which this "
+                                     "frame's properties are merged before "
+                                     "applying. This is used to unify "
+                                     "properties and avoid needing to specify "
+                                     "the same values for the same properties "
+                                     "in multiple frames.",
+                      "role": "info",
+                      "valType": "string"
+                  },
+                  "data": {
+                      "description": "A list of traces this frame modifies. "
+                                     "The format is identical to the normal "
+                                     "trace definition.",
+                      "role": "object",
+                      "valType": "any"
+                  },
+                  "group": {
+                      "description": "An identifier that specifies the group "
+                                     "to which the frame belongs, used by "
+                                     "animate to select a subset of frames.",
+                      "role": "info",
+                      "valType": "string"
+                  },
+                  "layout": {
+                      "role": "object",
+                      "description": "Layout properties which this frame "
+                                     "modifies. The format is identical to "
+                                     "the normal layout definition.",
+                      "valType": "any"
+                  },
+                  "name": {
+                      "description": "A label by which to identify the frame",
+                      "role": "info",
+                      "valType": "string"
+                  },
+                  "role": "object",
+                  "traces": {
+                      "description": "A list of trace indices that identify "
+                                     "the respective traces in the data "
+                                     "attribute",
+                      "role": "info",
+                      "valType": "info_array"
+                  }
+              }
+          },
+          "role": "object"
+    }
 
-    return utils.decode_unicode(graph_reference)
+    return graph_reference
 
 
 def string_to_class_name(string):
@@ -136,6 +189,27 @@ def get_attributes_dicts(object_name, parent_object_names=()):
     # We should also one or more paths where attributes are defined.
     attribute_paths = list(object_dict['attribute_paths'])  # shallow copy
 
+    # Map frame 'data' and 'layout' to previously-defined figure attributes.
+    # Examples of parent_object_names changes:
+    #   ['figure', 'frames'] --> ['figure', 'frames']
+    #   ['figure', 'frames', FRAME_NAME] --> ['figure']
+    #   ['figure', 'frames', FRAME_NAME, 'data'] --> ['figure', 'data']
+    #   ['figure', 'frames', FRAME_NAME, 'layout'] --> ['figure', 'layout']
+    #   ['figure', 'frames', FRAME_NAME, 'foo'] -->
+    #     ['figure', 'frames', FRAME_NAME, 'foo']
+    #   [FRAME_NAME, 'layout'] --> ['figure', 'layout']
+    if FRAME_NAME in parent_object_names:
+        len_parent_object_names = len(parent_object_names)
+        index = parent_object_names.index(FRAME_NAME)
+        if len_parent_object_names == index + 1:
+            if object_name in ('data', 'layout'):
+                parent_object_names = ['figure', object_name]
+        elif len_parent_object_names > index + 1:
+            if parent_object_names[index + 1] in ('data', 'layout'):
+                parent_object_names = (
+                    ['figure'] + list(parent_object_names)[index + 1:]
+                )
+
     # If we have parent_names, some of these attribute paths may be invalid.
     for parent_object_name in reversed(parent_object_names):
         if parent_object_name in ARRAYS:
@@ -410,8 +484,11 @@ def _patch_objects():
                          'attribute_paths': layout_attribute_paths,
                          'additional_attributes': {}}
 
-    figure_attributes = {'layout': {'role': 'object'},
-                         'data': {'role': 'object', '_isLinkedToArray': True}}
+    figure_attributes = {
+        'layout': {'role': 'object'},
+        'data': {'role': 'object', '_isLinkedToArray': True},
+        'frames': {'role': 'object', '_isLinkedToArray': True}
+    }
     OBJECTS['figure'] = {'meta_paths': [],
                          'attribute_paths': [],
                          'additional_attributes': figure_attributes}
@@ -479,6 +556,8 @@ def _get_classes():
 # The ordering here is important.
 GRAPH_REFERENCE = get_graph_reference()
 
+FRAME_NAME = list(GRAPH_REFERENCE['frames']['items'].keys())[0]
+
 # See http://blog.labix.org/2008/06/27/watch-out-for-listdictkeys-in-python-3
 TRACE_NAMES = list(GRAPH_REFERENCE['traces'].keys())
 
diff --git a/plotly/package_data/default-schema.json b/plotly/package_data/default-schema.json
index 4b0a3870b..c6f24733f 100644
--- a/plotly/package_data/default-schema.json
+++ b/plotly/package_data/default-schema.json
@@ -9136,7 +9136,7 @@
                         ]
                     },
                     "end": {
-                        "description": "Sets the end contour level value.",
+                        "description": "Sets the end contour level value. Must be more than `contours.start`",
                         "dflt": null,
                         "role": "style",
                         "valType": "number"
@@ -9149,13 +9149,14 @@
                         "valType": "boolean"
                     },
                     "size": {
-                        "description": "Sets the step between each contour level.",
+                        "description": "Sets the step between each contour level. Must be positive.",
                         "dflt": null,
+                        "min": 0,
                         "role": "style",
                         "valType": "number"
                     },
                     "start": {
-                        "description": "Sets the starting contour level value.",
+                        "description": "Sets the starting contour level value. Must be less than `contours.end`",
                         "dflt": null,
                         "role": "style",
                         "valType": "number"
@@ -9240,8 +9241,9 @@
                     "valType": "string"
                 },
                 "ncontours": {
-                    "description": "Sets the maximum number of contour levels. The actual number of contours will be chosen automatically to be less than or equal to the value of `ncontours`. Has an effect only if `autocontour` is *true*.",
-                    "dflt": 0,
+                    "description": "Sets the maximum number of contour levels. The actual number of contours will be chosen automatically to be less than or equal to the value of `ncontours`. Has an effect only if `autocontour` is *true* or if `contours.size` is missing.",
+                    "dflt": 15,
+                    "min": 1,
                     "role": "style",
                     "valType": "integer"
                 },
@@ -12754,7 +12756,7 @@
                         ]
                     },
                     "end": {
-                        "description": "Sets the end contour level value.",
+                        "description": "Sets the end contour level value. Must be more than `contours.start`",
                         "dflt": null,
                         "role": "style",
                         "valType": "number"
@@ -12767,13 +12769,14 @@
                         "valType": "boolean"
                     },
                     "size": {
-                        "description": "Sets the step between each contour level.",
+                        "description": "Sets the step between each contour level. Must be positive.",
                         "dflt": null,
+                        "min": 0,
                         "role": "style",
                         "valType": "number"
                     },
                     "start": {
-                        "description": "Sets the starting contour level value.",
+                        "description": "Sets the starting contour level value. Must be less than `contours.end`",
                         "dflt": null,
                         "role": "style",
                         "valType": "number"
@@ -12899,8 +12902,9 @@
                     "valType": "integer"
                 },
                 "ncontours": {
-                    "description": "Sets the maximum number of contour levels. The actual number of contours will be chosen automatically to be less than or equal to the value of `ncontours`. Has an effect only if `autocontour` is *true*.",
-                    "dflt": 0,
+                    "description": "Sets the maximum number of contour levels. The actual number of contours will be chosen automatically to be less than or equal to the value of `ncontours`. Has an effect only if `autocontour` is *true* or if `contours.size` is missing.",
+                    "dflt": 15,
+                    "min": 1,
                     "role": "style",
                     "valType": "integer"
                 },
diff --git a/update_graph_objs.py b/update_graph_objs.py
index 71882c8d5..0414dd0c1 100644
--- a/update_graph_objs.py
+++ b/update_graph_objs.py
@@ -1,3 +1,5 @@
+from __future__ import print_function
+
 from plotly.graph_objs import graph_objs_tools
 from plotly.graph_reference import ARRAYS, CLASSES
 
@@ -35,22 +37,7 @@ def print_figure_patch(f):
     def __init__(self, *args, **kwargs):
         super(Figure, self).__init__(*args, **kwargs)
         if 'data' not in self:
-            self.data = GraphObjectFactory.create('data', _parent=self,
-                                                  _parent_key='data')
-
-    # TODO better integrate frames into Figure - #604
-    def __setitem__(self, key, value, **kwargs):
-        if key == 'frames':
-            super(PlotlyDict, self).__setitem__(key, value)
-        else:
-            super(Figure, self).__setitem__(key, value, **kwargs)
-
-    def _get_valid_attributes(self):
-        super(Figure, self)._get_valid_attributes()
-        # TODO better integrate frames into Figure - #604
-        if 'frames' not in self._valid_attributes:
-            self._valid_attributes.add('frames')
-        return self._valid_attributes
+            self.data = Data(_parent=self, _parent_key='data')
 
     def get_data(self, flatten=False):
         """
@@ -221,6 +208,45 @@ def print_data_patch(f):
     )
 
 
+def print_frames_patch(f):
+    """Print a patch to our Frames object into the given open file."""
+    print(
+        '''
+    def _value_to_graph_object(self, index, value, _raise=True):
+        if isinstance(value, six.string_types):
+            return value
+        return super(Frames, self)._value_to_graph_object(index, value,
+                                                          _raise=_raise)
+
+    def to_string(self, level=0, indent=4, eol='\\n',
+                  pretty=True, max_chars=80):
+        """Get formatted string by calling `to_string` on children items."""
+        if not len(self):
+            return "{name}()".format(name=self._get_class_name())
+        string = "{name}([{eol}{indent}".format(
+            name=self._get_class_name(),
+            eol=eol,
+            indent=' ' * indent * (level + 1))
+        for index, entry in enumerate(self):
+            if isinstance(entry, six.string_types):
+                string += repr(entry)
+            else:
+                string += entry.to_string(level=level+1,
+                                          indent=indent,
+                                          eol=eol,
+                                          pretty=pretty,
+                                          max_chars=max_chars)
+            if index < len(self) - 1:
+                string += ",{eol}{indent}".format(
+                    eol=eol,
+                    indent=' ' * indent * (level + 1))
+        string += (
+            "{eol}{indent}])").format(eol=eol, indent=' ' * indent * level)
+        return string
+''', file=f, end=''
+    )
+
+
 def print_class(name, f):
     class_dict = CLASSES[name]
     print('\n', file=f)
@@ -250,6 +276,8 @@ def print_class(name, f):
         print_figure_patch(f)
     elif name == 'Data':
         print_data_patch(f)
+    elif name == 'Frames':
+        print_frames_patch(f)
 
 copied_lines = get_non_generated_file_lines()
 with open('./plotly/graph_objs/graph_objs.py', 'w') as graph_objs_file:


Test patch:
diff --git a/plotly/tests/test_core/test_graph_objs/test_figure.py b/plotly/tests/test_core/test_graph_objs/test_figure.py
new file mode 100644
index 000000000..6bf007c4c
--- /dev/null
+++ b/plotly/tests/test_core/test_graph_objs/test_figure.py
@@ -0,0 +1,37 @@
+from __future__ import absolute_import
+
+from unittest import TestCase
+
+from plotly import exceptions
+from plotly.graph_objs import Figure
+
+
+class FigureTest(TestCase):
+
+    def test_instantiation(self):
+
+        native_figure = {
+            'data': [],
+            'layout': {},
+            'frames': []
+        }
+
+        Figure(native_figure)
+        Figure()
+
+    def test_access_top_level(self):
+
+        # Figure is special, we define top-level objects that always exist.
+
+        self.assertEqual(Figure().data, [])
+        self.assertEqual(Figure().layout, {})
+        self.assertEqual(Figure().frames, [])
+
+    def test_nested_frames(self):
+        with self.assertRaisesRegexp(exceptions.PlotlyDictKeyError, 'frames'):
+            Figure({'frames': [{'frames': []}]})
+
+        figure = Figure()
+        figure.frames = [{}]
+        with self.assertRaisesRegexp(exceptions.PlotlyDictKeyError, 'frames'):
+            figure.frames[0].frames = []
diff --git a/plotly/tests/test_core/test_graph_objs/test_frames.py b/plotly/tests/test_core/test_graph_objs/test_frames.py
new file mode 100644
index 000000000..2b105c6ec
--- /dev/null
+++ b/plotly/tests/test_core/test_graph_objs/test_frames.py
@@ -0,0 +1,79 @@
+from __future__ import absolute_import
+
+from unittest import TestCase
+
+from plotly import exceptions
+from plotly.graph_objs import Bar, Frames
+
+
+class FramesTest(TestCase):
+
+    def test_instantiation(self):
+
+        native_frames = [
+            {},
+            {'data': []},
+            'foo',
+            {'data': [], 'group': 'baz', 'layout': {}, 'name': 'hoopla'}
+        ]
+
+        Frames(native_frames)
+        Frames()
+
+    def test_string_frame(self):
+        frames = Frames()
+        frames.append({'group': 'baz', 'data': []})
+        frames.append('foobar')
+        self.assertEqual(frames[1], 'foobar')
+        self.assertEqual(frames.to_string(),
+                         "Frames([\n"
+                         "    dict(\n"
+                         "        data=Data(),\n"
+                         "        group='baz'\n"
+                         "    ),\n"
+                         "    'foobar'\n"
+                         "])")
+
+    def test_non_string_frame(self):
+        frames = Frames()
+        frames.append({})
+
+        with self.assertRaises(exceptions.PlotlyListEntryError):
+            frames.append([])
+
+        with self.assertRaises(exceptions.PlotlyListEntryError):
+            frames.append(0)
+
+    def test_deeply_nested_layout_attributes(self):
+        frames = Frames()
+        frames.append({})
+        frames[0].layout.xaxis.showexponent = 'all'
+
+        # It's OK if this needs to change, but we should check *something*.
+        self.assertEqual(
+            frames[0].layout.font._get_valid_attributes(),
+            {'color', 'family', 'size'}
+        )
+
+    def test_deeply_nested_data_attributes(self):
+        frames = Frames()
+        frames.append({})
+        frames[0].data = [Bar()]
+        frames[0].data[0].marker.color = 'red'
+
+        # It's OK if this needs to change, but we should check *something*.
+        self.assertEqual(
+            frames[0].data[0].marker.line._get_valid_attributes(),
+            {'colorsrc', 'autocolorscale', 'cmin', 'colorscale', 'color',
+             'reversescale', 'width', 'cauto', 'widthsrc', 'cmax'}
+        )
+
+    def test_frame_only_attrs(self):
+        frames = Frames()
+        frames.append({})
+
+        # It's OK if this needs to change, but we should check *something*.
+        self.assertEqual(
+            frames[0]._get_valid_attributes(),
+            {'group', 'name', 'data', 'layout', 'baseframe', 'traces'}
+        )
diff --git a/plotly/tests/test_core/test_graph_reference/test_graph_reference.py b/plotly/tests/test_core/test_graph_reference/test_graph_reference.py
index 84fb8720f..f3f97c7c5 100644
--- a/plotly/tests/test_core/test_graph_reference/test_graph_reference.py
+++ b/plotly/tests/test_core/test_graph_reference/test_graph_reference.py
@@ -20,16 +20,6 @@ from plotly.tests.utils import PlotlyTestCase
 
 class TestGraphReferenceCaching(PlotlyTestCase):
 
-    def test_get_graph_reference(self):
-
-        # if we don't have a graph reference we load an outdated default
-
-        path = os.path.join('package_data', 'default-schema.json')
-        s = resource_string('plotly', path).decode('utf-8')
-        default_graph_reference = json.loads(s)
-        graph_reference = gr.get_graph_reference()
-        self.assertEqual(graph_reference, default_graph_reference)
-
     @attr('slow')
     def test_default_schema_is_up_to_date(self):
         api_domain = files.FILE_CONTENT[files.CONFIG_FILE]['plotly_api_domain']