Cannot inherit DataArray anymore in 0.7 release

[rafa-guedes]

I understand from @shoyer that inheriting from DataArray may not be the best approach to extend DataArray with other specific methods but this was working before the latest release, and it is not working anymore. Just wondering if this would be some issue caused by new internal structure of DataArray, or maybe something I'm doing wrong?

For example, the code below works using xray.0.6.1:

import numpy as np
# import xarray as xr #xarray.0.7.0
import xray as xr #xray.0.6.1

class NewArray(xr.DataArray):
    def __init__(self, darray):
        super(NewArray, self).__init__(darray, name='spec')

data = np.random.randint(0,10,12).reshape(4,3)
x = [10,20,30]
y = [1,2,3,4]

darray = xr.DataArray(data, coords={'y': y, 'x': x}, dims=['y','x'])
narray = NewArray(darray)

print 'xr version: %s\n' % xr.__version__
print 'DataArray object:\n%s\n' % darray
print 'NewArray object:\n%s' % narray

but it does not work anymore when using the new xarray release. The NewArray instance is actually created, but if I try to access this object, or its narray.coords attribute, I get the traceback below. I can however access some other attributes from narray such as narray.values or narray.dims.

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/source/pymsl/pymsl/core/tests/inherit_test.py in <module>()
     16 print 'xr version: %s\n' % xr.__version__
     17 print 'DataArray object:\n%s\n' % darray
---> 18 print 'NewArray object:\n%s' % narray

/usr/local/lib/python2.7/site-packages/xarray/core/common.pyc in __repr__(self)
     76
     77     def __repr__(self):
---> 78         return formatting.array_repr(self)
     79
     80     def _iter(self):

/usr/local/lib/python2.7/site-packages/xarray/core/formatting.pyc in array_repr(arr)
    254     if hasattr(arr, 'coords'):
    255         if arr.coords:
--> 256             summary.append(repr(arr.coords))
    257
    258     if arr.attrs:

/usr/local/lib/python2.7/site-packages/xarray/core/coordinates.pyc in __repr__(self)
     64
     65     def __repr__(self):
---> 66         return formatting.coords_repr(self)
     67
     68     @property

/usr/local/lib/python2.7/site-packages/xarray/core/formatting.pyc in _mapping_repr(mapping, title, summarizer, col_width)
    208     summary = ['%s:' % title]
    209     if mapping:
--> 210         summary += [summarizer(k, v, col_width) for k, v in mapping.items()]
    211     else:
    212         summary += [EMPTY_REPR]

/usr/local/Cellar/python/2.7.10_2/Frameworks/Python.framework/Versions/2.7/lib/python2.7/_abcoll.pyc in items(self)
    412     def items(self):
    413         "D.items() -> list of D's (key, value) pairs, as 2-tuples"
--> 414         return [(key, self[key]) for key in self]
    415
    416     def values(self):

/usr/local/lib/python2.7/site-packages/xarray/core/coordinates.pyc in __getitem__(self, key)
     44              key.split('.')[0] in self._names)):
     45             # allow indexing current coordinates or components
---> 46             return self._data[key]
     47         else:
     48             raise KeyError(key)

/usr/local/lib/python2.7/site-packages/xarray/core/dataarray.pyc in __getitem__(self, key)
    395                 _, key, var = _get_virtual_variable(self._coords, key)
    396
--> 397             return self._replace_maybe_drop_dims(var, name=key)
    398         else:
    399             # orthogonal array indexing

/usr/local/lib/python2.7/site-packages/xarray/core/dataarray.pyc in _replace_maybe_drop_dims(self, variable, name)
    234             coords = OrderedDict((k, v) for k, v in self._coords.items()
    235                                  if set(v.dims) <= allowed_dims)
--> 236         return self._replace(variable, coords, name)
    237
    238     __this_array = _ThisArray()

/usr/local/lib/python2.7/site-packages/xarray/core/dataarray.pyc in _replace(self, variable, coords, name)
    225         if name is self.__default:
    226             name = self.name
--> 227         return type(self)(variable, coords, name=name, fastpath=True)
    228
    229     def _replace_maybe_drop_dims(self, variable, name=__default):

TypeError: __init__() takes exactly 2 arguments (5 given)

[shoyer]

This was definitely caused by the refactor of DataArray internals in 0.7.0.

I don't think you were doing anything wrong, per se, it's just that the internal refactor changed what you can do with a subclasses in an unexpected way :(.

On the plus side, the new code should be better about preserving DataArray subclasses -- new DataArrays should be to the same type as parent objects. However, you will need to preserve the signature of DataArray.__init__, or replace the private _replace method on your subclass (I don't recommend this, because it's also definitely private API and not guaranteed to continue to work).

[rafa-guedes]

Thanks @shoyer ,
what do you mean by preserve the signature of DataArray.__init__ please?

[shoyer]

@rafa-guedes the __init__ method needs to look like that of DataArray, e.g.,

class NewArray(xr.DataArray):
    def __init__(self, data, coords=None, dims=None, name=None, attrs=None, encoding=None, fastpath=False):
        # ...

or even simply:

class NewArray(xr.DataArray):
    def __init__(self, *args, **kwargs):
        # ...

[rafa-guedes]

Thanks @shoyer that works (:

[MathieuSchopfer]

@shoyer Then it is not OOP any more ...

[shoyer]

@mathieuS87 I'm not quite sure what you mean there. Do you have a suggested alternative?

[MathieuSchopfer]

Well, I think pure OOP should support class inheritance. In this case, you can subclass DataArray only if you do not customise the __init__ method, which is not true inheritance ...

Yes, I have an alternative to suggest: initialise subclass instances in two steps, by creating a public method init(self, ...) or initialise(self, ...) to set custom attributes.

[max-sixty]

Well, I think pure OOP should support class inheritance. In this case, you can subclass DataArray only if you do not customise the init method, which is not true inheritance ...

That's not the pythonic perspective (although I can see the attraction). The pythonic way is to override __init__ and super up

[jhamman]

Also, even in OOP, I'm not sure every class needs to be designed to support inheritance. There are obvious applications where that is not practical.

[MathieuSchopfer]

@MaximilianR Okay, but this won't solve the current issue. If you override the __init__ method when trying to subclass DataArray, subsequent accesses to some attributes (e.g. coords) will fail. No matter if you super up or not.

[max-sixty]

@MaximilianR Okay, but this won't solve the current issue. If you override the init method when trying to subclass DataArray, subsequent accesses to some attributes (e.g. coords) will fail. No matter if you super up or not.

Why?

[MathieuSchopfer]

That's the point of this issue, why. Read this comment of @shoyer.

[mivade]

This is a really painful bug since it leads to all sorts of obscure error messages. Is there a reason inheritance with a different __init__ signature can't be allowed? At the very least, a big fat warning in the documentation would be helpful.

[shoyer]

Is there a reason inheritance with a different init signature can't be allowed?

No strong reason -- we just didn't really consider (or test) inheritance in the current design. Certainly proposals (and PRs) for a subclassing API would be welcome.

At the very least, a big fat warning in the documentation would be helpful.

http://xarray.pydata.org/en/stable/internals.html#extending-xarray has some related guidance (I agree it could be louder)