Use explicit type check in as_compatible_data instead of blanket access to values attribute (#2905)

* In as_compatible_data, check explicitly for nested self-described types

This change was prompted by the fact that `getattr(data, 'values', data)`
affected any kind of data with a `values` attribute, which is not the
desired behaviour at that point.

This also extends tests to assert that custom objects with such an
attribute are not attempted to be converted

* Add whats-new entry

* Remove trailing whitespace

* In as_compatible_data, check explicitly for nested self-described types

This change was prompted by the fact that `getattr(data, 'values', data)`
affected any kind of data with a `values` attribute, which is not the
desired behaviour at that point.

This also extends tests to assert that custom objects with such an
attribute are not attempted to be converted

* whats-new

* Fix test.

* Update @blsqr github URL in whats-new

* actually check that values is not extracted

Co-authored-by: dcherian <[email protected]>
Co-authored-by: Keewis <[email protected]>

Author: 3 people

doc/whats-new.rst

@@ -90,6 +90,10 @@ New Features
 
 Bug fixes
 ~~~~~~~~~
+- Use specific type checks in
+  :py:func:`~xarray.core.variable.as_compatible_data` instead of blanket
+  access to ``values`` attribute (:issue:`2097`)
+  By `Yunus Sevinchan <https://github.com/blsqr>`_.
 - :py:meth:`DataArray.resample` and :py:meth:`Dataset.resample` do not trigger computations anymore if :py:meth:`Dataset.weighted` or :py:meth:`DataArray.weighted` are applied (:issue:`4625`, :pull:`4668`). By `Julius Busecke <https://github.com/jbusecke>`_.
 - :py:func:`merge` with ``combine_attrs='override'`` makes a copy of the attrs (:issue:`4627`).
 - By default, when possible, xarray will now always use values of type ``int64`` when encoding

xarray/core/variable.py

@@ -218,7 +218,8 @@ def as_compatible_data(data, fastpath=False):
         data = np.timedelta64(getattr(data, "value", data), "ns")
 
     # we don't want nested self-described arrays
-    data = getattr(data, "values", data)
+    if isinstance(data, (pd.Series, pd.Index, pd.DataFrame)):
+        data = data.values
 
     if isinstance(data, np.ma.MaskedArray):
         mask = np.ma.getmaskarray(data)

xarray/tests/test_variable.py

@@ -2300,6 +2300,11 @@ def __init__(self, array):
         class CustomIndexable(CustomArray, indexing.ExplicitlyIndexed):
             pass
 
+        # Type with data stored in values attribute
+        class CustomWithValuesAttr:
+            def __init__(self, array):
+                self.values = array
+
         array = CustomArray(np.arange(3))
         orig = Variable(dims=("x"), data=array, attrs={"foo": "bar"})
         assert isinstance(orig._data, np.ndarray)  # should not be CustomArray
@@ -2308,6 +2313,10 @@ class CustomIndexable(CustomArray, indexing.ExplicitlyIndexed):
         orig = Variable(dims=("x"), data=array, attrs={"foo": "bar"})
         assert isinstance(orig._data, CustomIndexable)
 
+        array = CustomWithValuesAttr(np.arange(3))
+        orig = Variable(dims=(), data=array)
+        assert isinstance(orig._data.item(), CustomWithValuesAttr)
+
 
 def test_raise_no_warning_for_nan_in_binary_ops():
     with pytest.warns(None) as record: