Merge pull request #715 from shoyer/setattr-hard-fail

Error when attempting to assign using attribute style syntax

Author: shoyer

doc/whats-new.rst

@@ -66,6 +66,9 @@ Bug fixes
 
 - Fixes for several issues found on ``DataArray`` objects with the same name
   as one of their coordinates (see :ref:`v0.7.0.breaking` for more details).
+- Attempting to assign a ``Dataset`` or ``DataArray`` variable/attribute using
+  attribute-style syntax (e.g., ``ds.foo = 42``) now raises an error rather
+  than silently failing (:issue:`656`, :issue:`714`).
 
 - ``DataArray.to_masked_array`` always returns masked array with mask being an array
 (not a scalar value) (:issue:`684`)

xray/core/common.py

@@ -117,8 +117,10 @@ def _get_axis_num(self, dim):
 
 
 class AttrAccessMixin(object):
-    """Mixin class that allow getting keys with attribute access
+    """Mixin class that allows getting keys with attribute access
     """
+    _initialized = False
+
     @property
     def _attr_sources(self):
         """List of places to look-up items for attribute-style access"""
@@ -134,6 +136,20 @@ def __getattr__(self, name):
         raise AttributeError("%r object has no attribute %r" %
                              (type(self).__name__, name))
 
+    def __setattr__(self, name, value):
+        if self._initialized:
+            try:
+                # Allow setting instance variables if they already exist
+                # (e.g., _attrs). We use __getattribute__ instead of hasattr
+                # to avoid key lookups with attribute-style access.
+                self.__getattribute__(name)
+            except AttributeError:
+                raise AttributeError(
+                    "cannot set attribute %r on a %r object. Use __setitem__ "
+                    "style assignment (e.g., `ds['name'] = ...`) instead to "
+                    "assign variables." % (name, type(self).__name__))
+        object.__setattr__(self, name, value)
+
     def __dir__(self):
         """Provide method name lookup and completion. Only provide 'public'
         methods.

xray/core/dataarray.py

@@ -213,6 +213,7 @@ def __init__(self, data, coords=None, dims=None, name=None,
         self._variable = variable
         self._coords = coords
         self._name = name
+        self._initialized = True
 
     __default = object()
 

xray/core/dataset.py

@@ -209,6 +209,7 @@ def __init__(self, data_vars=None, coords=None, attrs=None,
             self._set_init_vars_and_dims(data_vars, coords, compat)
         if attrs is not None:
             self.attrs = attrs
+        self._initialized = True
 
     def _add_missing_coords_inplace(self):
         """Add missing coordinates to self._variables
@@ -370,6 +371,7 @@ def _construct_direct(cls, variables, coord_names, dims, attrs,
         obj._dims = dims
         obj._attrs = attrs
         obj._file_obj = file_obj
+        obj._initialized = True
         return obj
 
     __default_attrs = object()

xray/test/test_dataarray.py

@@ -1558,3 +1558,12 @@ def test_real_and_imag(self):
         array = DataArray(1 + 2j)
         self.assertDataArrayIdentical(array.real, DataArray(1))
         self.assertDataArrayIdentical(array.imag, DataArray(2))
+
+    def test_setattr_raises(self):
+        array = DataArray(0, coords={'scalar': 1}, attrs={'foo': 'bar'})
+        with self.assertRaisesRegexp(AttributeError, 'cannot set attr'):
+            array.scalar = 2
+        with self.assertRaisesRegexp(AttributeError, 'cannot set attr'):
+            array.foo = 2
+        with self.assertRaisesRegexp(AttributeError, 'cannot set attr'):
+            array.other = 2

xray/test/test_dataset.py

@@ -2289,3 +2289,12 @@ def test_real_and_imag(self):
 
         expected_im = Dataset({'x': ((), 2, attrs)}, attrs=attrs)
         self.assertDatasetIdentical(ds.imag, expected_im)
+
+    def test_setattr_raises(self):
+        ds = Dataset({}, coords={'scalar': 1}, attrs={'foo': 'bar'})
+        with self.assertRaisesRegexp(AttributeError, 'cannot set attr'):
+            ds.scalar = 2
+        with self.assertRaisesRegexp(AttributeError, 'cannot set attr'):
+            ds.foo = 2
+        with self.assertRaisesRegexp(AttributeError, 'cannot set attr'):
+            ds.other = 2