Warn instead of erroring when overriding attributes with accessors (#1149)

* Warn instead of erroring when overriding attributes with accessors

Fixes GH1080, GH1082

CC smartass101 rabernat

* use a custom warning class

* grammar

Author: shoyer

doc/internals.rst

@@ -54,7 +54,7 @@ Extending xarray
     np.random.seed(123456)
 
 xarray is designed as a general purpose library, and hence tries to avoid
-including overly domain specific methods. But inevitably, the need for more
+including overly domain specific functionality. But inevitably, the need for more
 domain specific logic arises.
 
 One standard solution to this problem is to subclass Dataset and/or DataArray to
@@ -69,7 +69,11 @@ task, even if most methods are only forwarding to xarray implementations.
 
 __ https://github.com/pydata/xarray/issues/706
 
-To resolve this dilemma, xarray has the experimental
+If you simply want the ability to call a function with the syntax of a
+method call, then the builtin :py:meth:`~xarray.DataArray.pipe` method (copied
+from pandas) may suffice.
+
+To resolve this issue for more complex cases, xarray has the
 :py:func:`~xarray.register_dataset_accessor` and
 :py:func:`~xarray.register_dataarray_accessor` decorators for adding custom
 "accessors" on xarray objects. Here's how you might use these decorators to
@@ -90,14 +94,17 @@ defined that returns an instance of your class:
           return GeoAccessor(self)
 
 However, using the register accessor decorators is preferable to simply adding
-your own ad-hoc property (i.e., ``Dataset.geo = property(...)``), for two
+your own ad-hoc property (i.e., ``Dataset.geo = property(...)``), for several
 reasons:
 
-1. It ensures that the name of your property does not conflict with any other
-   attributes or methods.
+1. It ensures that the name of your property does not accidentally conflict with
+   any other attributes or methods (including other accessors).
 2. Instances of accessor object will be cached on the xarray object that creates
    them. This means you can save state on them (e.g., to cache computed
    properties).
+3. Using an accessor provides an implicit namespace for your custom
+   functionality that clearly identifies it as separate from built-in xarray
+   methods.
 
 Back in an interactive IPython session, we can use these properties:
 
@@ -115,7 +122,9 @@ Back in an interactive IPython session, we can use these properties:
 
 The intent here is that libraries that extend xarray could add such an accessor
 to implement subclass specific functionality rather than using actual subclasses
-or patching in a large number of domain specific methods.
+or patching in a large number of domain specific methods. For further reading
+on ways to write new accessors and the philosophy behind the approach, see
+:issue:`1080`.
 
 To help users keep things straight, please `let us know
 <https://github.com/pydata/xarray/issues>`_ if you plan to write a new accessor

doc/whats-new.rst

@@ -104,6 +104,11 @@ Enhancements
 - New top-level functions :py:func:`~xarray.full_like`,
   :py:func:`~xarray.zeros_like`, and :py:func:`~xarray.ones_like`
   By `Guido Imperiale <https://github.com/crusaderky>`_.
+- Overriding a preexisting attribute with
+  :py:func:`~xarray.register_dataset_accessor` or
+  :py:func:`~xarray.register_dataarray_accessor` now issues a warning instead of
+  raising an error (:issue:`1082`).
+  By `Stephan Hoyer <https://github.com/shoyer>`_.
 
 Bug fixes
 ~~~~~~~~~

xarray/core/extensions.py

@@ -2,14 +2,15 @@
 from __future__ import division
 from __future__ import print_function
 import traceback
+import warnings
 
 from .dataarray import DataArray
 from .dataset import Dataset
 from .pycompat import PY2
 
 
-class AccessorRegistrationError(Exception):
-    """Exception for conflicts in accessor registration."""
+class AccessorRegistrationWarning(Warning):
+    """Warning for conflicts in accessor registration."""
 
 
 class _CachedAccessor(object):
@@ -43,10 +44,12 @@ def __get__(self, obj, cls):
 def _register_accessor(name, cls):
     def decorator(accessor):
         if hasattr(cls, name):
-            raise AccessorRegistrationError(
-                'cannot register accessor %r under name %r for type %r '
-                'because an attribute with that name already exists.'
-                % (accessor, name, cls))
+            warnings.warn(
+                'registration of accessor %r under name %r for type %r is '
+                'overriding a preexisting attribute with the same name.'
+                % (accessor, name, cls),
+                AccessorRegistrationWarning,
+                stacklevel=2)
         setattr(cls, name, _CachedAccessor(name, accessor))
         return accessor
     return decorator
@@ -58,7 +61,8 @@ def register_dataarray_accessor(name):
     Parameters
     ----------
     name : str
-        Name under which the accessor should be registered.
+        Name under which the accessor should be registered. A warning is issued
+        if this name conflicts with a preexisting attribute.
 
     Examples
     --------
@@ -105,7 +109,8 @@ def register_dataset_accessor(name):
     Parameters
     ----------
     name : str
-        Name under which the property should be registered.
+        Name under which the accessor should be registered. A warning is issued
+        if this name conflicts with a preexisting attribute.
 
     See also
     --------

xarray/test/test_extensions.py

@@ -52,7 +52,7 @@ def foo(self):
         del xr.Dataset.demo
         assert not hasattr(xr.Dataset, 'demo')
 
-        with self.assertRaises(xr.core.extensions.AccessorRegistrationError):
+        with self.assertWarns('overriding a preexisting attribute'):
             @xr.register_dataarray_accessor('demo')
             class Foo(object):
                 pass