Repository: altair-viz/altair
Pull request #409 (2017-10-17 03:32:33)

ENH to_dict(): clone chart object before mutation
As discussed today with @ellisonbg.

This makes it so that exporting and/or visualizing the chart doesn't silently modify the object, but rather creates a clone of the chart before modification.


Patch:
diff --git a/altair/v1/api.py b/altair/v1/api.py
index 1b0ecea0..925d2dfa 100644
--- a/altair/v1/api.py
+++ b/altair/v1/api.py
@@ -268,7 +268,7 @@ class TopLevelMixin(object):
         spec : dict
             The JSON specification of the chart object.
         """
-        dct = super(TopLevelMixin, self).to_dict(data=data)
+        dct = super(TopLevelMixin, self.clone()).to_dict(data=data)
         dct['$schema'] = schema.vegalite_schema_url
         return dct
 
@@ -528,6 +528,14 @@ class Chart(TopLevelMixin, schema.ExtendedUnitSpec):
         help="Maximum number of rows in the dataset to accept."
     )
 
+    def clone(self):
+        """
+        Return a clone of this object, recursively cloning each trait
+        """
+        copy = super(Chart, self).clone()
+        copy._data = self._data
+        return copy
+
     @property
     def data(self):
         return self._data
@@ -675,6 +683,14 @@ class LayeredChart(TopLevelMixin, schema.LayerSpec):
         help="Maximum number of rows in the dataset to accept."
     )
 
+    def clone(self):
+        """
+        Return a clone of this object, recursively cloning each trait
+        """
+        copy = super(LayeredChart, self).clone()
+        copy._data = self._data
+        return copy
+
     @property
     def data(self):
         return self._data
@@ -732,6 +748,14 @@ class FacetedChart(TopLevelMixin, schema.FacetSpec):
         help="Maximum number of rows in the dataset to accept."
     )
 
+    def clone(self):
+        """
+        Return a clone of this object, recursively cloning each trait
+        """
+        copy = super(FacetedChart, self).clone()
+        copy._data = self._data
+        return copy
+
     @property
     def data(self):
         return self._data
diff --git a/altair/v1/schema/_interface/__init__.py b/altair/v1/schema/_interface/__init__.py
index 7782f22e..7ea25363 100644
--- a/altair/v1/schema/_interface/__init__.py
+++ b/altair/v1/schema/_interface/__init__.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Auto-generated by schemapi: do not modify file directly
-# - schemapi version: 0.3.0.dev0+791c7f6
-# - date:    2017-10-11 11:25:24
+# - schemapi version: 0.3.0.dev0+e4f16a6
+# - date:    2017-10-16 20:17:34
 
 from .schema import Root
 from .schema import AggregateOp
diff --git a/altair/v1/schema/_interface/channel_collections.py b/altair/v1/schema/_interface/channel_collections.py
index 2302e0a9..3f321f58 100644
--- a/altair/v1/schema/_interface/channel_collections.py
+++ b/altair/v1/schema/_interface/channel_collections.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Auto-generated file: do not modify directly
-# - altair version info: v1.2.0-122-g9ac89eb
-# - date: 2017-10-11 11:25:24
+# - altair version info: v1.2.0-125-gaaa558d
+# - date: 2017-10-16 20:17:34
 
 import traitlets as T
 from . import jstraitlets as jst
diff --git a/altair/v1/schema/_interface/channel_wrappers.py b/altair/v1/schema/_interface/channel_wrappers.py
index 2fb4fff3..f7c1ec0a 100644
--- a/altair/v1/schema/_interface/channel_wrappers.py
+++ b/altair/v1/schema/_interface/channel_wrappers.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Auto-generated file: do not modify directly
-# - altair version info: v1.2.0-122-g9ac89eb
-# - date: 2017-10-11 11:25:24
+# - altair version info: v1.2.0-125-gaaa558d
+# - date: 2017-10-16 20:17:34
 
 import pandas as pd
 
diff --git a/altair/v1/schema/_interface/jstraitlets.py b/altair/v1/schema/_interface/jstraitlets.py
index e1d7c6b1..3ff7604d 100644
--- a/altair/v1/schema/_interface/jstraitlets.py
+++ b/altair/v1/schema/_interface/jstraitlets.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Auto-generated by schemapi: do not modify file directly
-# - schemapi version: 0.3.0.dev0+791c7f6
-# - date:    2017-10-11 11:25:24
+# - schemapi version: 0.3.0.dev0+e4f16a6
+# - date:    2017-10-16 20:17:34
 
 """
 Extensions to traitlets for compatibility with JSON Schema
@@ -29,6 +29,11 @@ from traitlets.utils.importstring import import_item
 __jsonschema_draft__ = 4
 
 
+class UndefinedTraitError(T.TraitError):
+    """Exception to raise when a required trait is undefined"""
+    pass
+
+
 class UndefinedType(object):
     """A Singleton type to mark undefined traits"""
     __instance = None
@@ -61,7 +66,7 @@ class JSONHasTraits(T.HasTraits):
     >>> sorted(f.trait_names())
     ['name', 'score', 'value']
     """
-    _additional_traits = True  # boolean, or list of
+    _additional_traits = True  # boolean, or length-1 list of trait type allowed
     _skip_on_export = []  # traits to skip when exporting to dictionary
     _required_traits = []  # traits required at export. If undefined, a traiterror will be raised
     _converter_registry = {}  # converter classes to use for to_dict, from_dict
@@ -97,6 +102,22 @@ class JSONHasTraits(T.HasTraits):
         return (key in self.traits() and
                 getattr(self, key, undefined) is not undefined)
 
+    def clone(self):
+        """
+        Return a clone of this object, recursively cloning each trait
+        """
+        def _clone(obj):
+            if isinstance(obj, JSONHasTraits):
+                return obj.clone()
+            elif isinstance(obj, list):
+                return [_clone(item) for item in obj]
+            else:
+                return obj
+
+        kwds = {name: _clone(getattr(self, name))
+                for name in self.trait_names()}
+        return self.__class__(**kwds)
+
     @classmethod
     def _get_additional_traits(cls):
         try:
@@ -636,6 +657,7 @@ class JSONNot(T.TraitType):
             self.error(obj, value)
 
 
+
 ##########################################################################
 # Visitor Pattern
 #
@@ -690,8 +712,8 @@ class ToDict(Visitor):
             if val is not undefined:
                 dct[schema_key] = self.visit(val, *args, **kwargs)
             elif traitlets_key in obj._required_traits:
-                raise T.TraitError("Required trait '{0}' is undefined'"
-                                   "".format(traitlets_key))
+                raise UndefinedTraitError("Required trait '{0}' is undefined "
+                                          "in {1}".format(traitlets_key, obj))
         for schema_key, val in obj._metadata.items():
             if val is not undefined and schema_key not in dct:
                 dct[schema_key] = val
@@ -805,7 +827,8 @@ class ToPython(Visitor):
 
         missing = set(obj._required_traits) - set(kwds)
         if missing:
-            raise T.TraitError("Required traits {0} missing".format(missing))
+            raise UndefinedTraitError("Required traits {0} missing "
+                                      "in {1}".format(missing, obj))
         arglist = '\n'.join('{0}={1},'.format(*item)
                             for item in sorted(kwds.items())).rstrip(',')
         return "{0}(\n{1}\n)".format(obj.__class__.__name__,
diff --git a/altair/v1/schema/_interface/named_channels.py b/altair/v1/schema/_interface/named_channels.py
index 84dd4526..d419cb7c 100644
--- a/altair/v1/schema/_interface/named_channels.py
+++ b/altair/v1/schema/_interface/named_channels.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Auto-generated file: do not modify directly
-# - altair version info: v1.2.0-122-g9ac89eb
-# - date: 2017-10-11 11:25:24
+# - altair version info: v1.2.0-125-gaaa558d
+# - date: 2017-10-16 20:17:34
 
 from . import channel_wrappers
 
diff --git a/altair/v1/schema/_interface/schema.py b/altair/v1/schema/_interface/schema.py
index efcdca14..009569fc 100644
--- a/altair/v1/schema/_interface/schema.py
+++ b/altair/v1/schema/_interface/schema.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Auto-generated by schemapi: do not modify file directly
-# - schemapi version: 0.3.0.dev0+791c7f6
-# - date:    2017-10-11 11:25:24
+# - schemapi version: 0.3.0.dev0+e4f16a6
+# - date:    2017-10-16 20:17:34
 
 
 
diff --git a/altair/v1/schema/visitors.py b/altair/v1/schema/visitors.py
index ce57005f..262ec33c 100644
--- a/altair/v1/schema/visitors.py
+++ b/altair/v1/schema/visitors.py
@@ -65,7 +65,8 @@ class ToPython(jst.ToPython):
                 getattr(obj, k, jst.undefined) is not jst.undefined}
         missing = set(obj._required_traits) - set(kwds)
         if missing:
-            raise T.TraitError("Required traits {0} missing".format(missing))
+            raise jst.UndefinedTraitError("Required traits {0} missing"
+                                          "".format(missing))
         kwds = {k: self.visit(v) for k, v in kwds.items()}
         return CodeGen(obj.__class__.__name__, kwargs=kwds)
 


Test patch:
diff --git a/altair/v1/schema/_interface/tests/__init__.py b/altair/v1/schema/_interface/tests/__init__.py
index 60cfbe68..4c92a8c0 100644
--- a/altair/v1/schema/_interface/tests/__init__.py
+++ b/altair/v1/schema/_interface/tests/__init__.py
@@ -1,5 +1,5 @@
 # -*- coding: utf-8 -*-
 # Auto-generated by schemapi: do not modify file directly
-# - schemapi version: 0.3.0.dev0+791c7f6
-# - date:    2017-10-11 11:25:24
+# - schemapi version: 0.3.0.dev0+e4f16a6
+# - date:    2017-10-16 20:17:34
 
diff --git a/altair/v1/schema/_interface/tests/test_jstraitlets.py b/altair/v1/schema/_interface/tests/test_jstraitlets.py
index e2b8c18b..80051fa7 100644
--- a/altair/v1/schema/_interface/tests/test_jstraitlets.py
+++ b/altair/v1/schema/_interface/tests/test_jstraitlets.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Auto-generated by schemapi: do not modify file directly
-# - schemapi version: 0.3.0.dev0+791c7f6
-# - date:    2017-10-11 11:25:24
+# - schemapi version: 0.3.0.dev0+e4f16a6
+# - date:    2017-10-16 20:17:34
 
 import pytest
 
@@ -14,6 +14,24 @@ def test_undefined_singleton():
     assert jst.UndefinedType() is undefined
 
 
+def test_undefined_trait_error():
+    class X(jst.JSONHasTraits):
+        _required_traits = ['foo']
+        foo = jst.JSONString()
+        bar = jst.JSONString()
+
+    # No error if foo is defined
+    X(foo='hello').to_dict()
+    X(foo='hello').to_python()
+
+    # Error if foo is undefined
+    with pytest.raises(jst.UndefinedTraitError):
+        X(bar='hello').to_dict()
+
+    with pytest.raises(jst.UndefinedTraitError):
+        X(bar='hello').to_python()
+
+
 def generate_test_cases():
     """yield tuples of (trait, failcases, passcases)"""
     # Anys
@@ -312,3 +330,29 @@ def test_trait_name_map():
     dct = {'for': 'abc', '$schema': 'http://www.schema.com/schema.json'}
     f = Foo.from_dict(dct)
     assert f.to_dict() == dct
+
+
+def test_clone():
+    class Name(jst.JSONHasTraits):
+        first = jst.JSONString()
+        last = jst.JSONString()
+
+    class NameCollection(jst.JSONHasTraits):
+        score = jst.JSONNumber()
+        names = jst.JSONArray(jst.JSONInstance(Name))
+
+    obj1 = NameCollection(score=42, names=[Name(first='sideshow', last='bob'),
+                                           Name(first='bart', last='simpson')])
+    D = obj1.to_dict()
+
+    # Make sure dicts match after clone
+    obj2 = obj1.clone()
+    assert obj2.to_dict() == D  # clone actually copies correctly
+
+    # Modify first object and make sure clone actually created a copy
+    obj1.score = 56
+    obj1.names[0].first = 'sponge'
+    obj1.names[1].last = 'train'
+
+    assert obj1.to_dict() != D  # original object is modified
+    assert obj2.to_dict() == D  # clone is not modified