Merge pull request #5859 from jreback/series_view

rework Series.sort is_view check on underlying ndarray inplace sort (GH5856)

Author: jreback

doc/source/release.rst

@@ -56,6 +56,9 @@ New features
 API Changes
 ~~~~~~~~~~~
 
+  - ``Series.sort`` will raise a ``ValueError`` (rather than a ``TypeError``) on sorting an
+    object that is a view of another (:issue:`5856`, :issue:`5853`)
+
 .. _release.bug_fixes-0.13.1:
 
 Experimental Features

pandas/core/frame.py

@@ -1604,10 +1604,15 @@ def _ixs(self, i, axis=0, copy=False):
                 values = self._data.iget(i)
                 if not len(values):
                     values = np.array([np.nan] * len(self.index), dtype=object)
-                return self._constructor_sliced.from_array(
+                result = self._constructor_sliced.from_array(
                     values, index=self.index,
                     name=label, fastpath=True)
 
+                # this is a cached value, mark it so
+                result._set_as_cached(i, self)
+
+                return result
+
     def iget_value(self, i, j):
         return self.iat[i, j]
 

pandas/core/generic.py

@@ -983,9 +983,14 @@ def _get_item_cache(self, item):
             values = self._data.get(item)
             res = self._box_item_values(item, values)
             cache[item] = res
-            res._cacher = (item, weakref.ref(self))
+            res._set_as_cached(item, self)
         return res
 
+    def _set_as_cached(self, item, cacher):
+        """ set the _cacher attribute on the calling object with
+            a weakref to cacher """
+        self._cacher = (item, weakref.ref(cacher))
+
     def _box_item_values(self, key, values):
         raise NotImplementedError
 
@@ -994,6 +999,12 @@ def _maybe_cache_changed(self, item, value):
         maybe it has changed """
         self._data.set(item, value)
 
+    @property
+    def _is_cached(self):
+        """ boolean : return if I am cached """
+        cacher = getattr(self, '_cacher', None)
+        return cacher is not None
+
     def _maybe_update_cacher(self, clear=False):
         """ see if we need to update our parent cacher
             if clear, then clear our cache """

pandas/core/series.py

@@ -1664,20 +1664,16 @@ def sort(self, axis=0, kind='quicksort', order=None, ascending=True):
         --------
         pandas.Series.order
         """
-        sortedSeries = self.order(na_last=True, kind=kind,
-                                  ascending=ascending)
 
-        true_base = self.values
-        while true_base.base is not None:
-            true_base = true_base.base
+        # GH 5856/5863
+        if self._is_cached:
+            raise ValueError("This Series is a view of some other array, to "
+                             "sort in-place you must create a copy")
 
-        if (true_base is not None and
-                (true_base.ndim != 1 or true_base.shape != self.shape)):
-            raise TypeError('This Series is a view of some other array, to '
-                            'sort in-place you must create a copy')
+        result = self.order(na_last=True, kind=kind,
+                            ascending=ascending)
 
-        self._data = sortedSeries._data.copy()
-        self.index = sortedSeries.index
+        self._update_inplace(result)
 
     def sort_index(self, ascending=True):
         """

pandas/tests/test_frame.py

@@ -9547,7 +9547,7 @@ def test_sort_datetimes(self):
 
     def test_frame_column_inplace_sort_exception(self):
         s = self.frame['A']
-        with assertRaisesRegexp(TypeError, "This Series is a view"):
+        with assertRaisesRegexp(ValueError, "This Series is a view"):
             s.sort()
 
         cp = s.copy()

pandas/tests/test_indexing.py

@@ -2019,6 +2019,20 @@ def f():
             zed['eyes']['right'].fillna(value=555, inplace=True)
         self.assertRaises(com.SettingWithCopyError, f)
 
+        # GH 5856/5863
+        # Series.sort operating on a view
+        df = DataFrame(np.random.randn(10,4))
+        s = df.iloc[:,0]
+        def f():
+            s.sort()
+        self.assertRaises(ValueError, f)
+
+        df = DataFrame(np.random.randn(10,4))
+        s = df.iloc[:,0]
+        s = s.order()
+        assert_series_equal(s,df.iloc[:,0].order())
+        assert_series_equal(s,df[0].order())
+
         pd.set_option('chained_assignment','warn')
 
     def test_float64index_slicing_bug(self):