Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 6, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/source/release.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
13 changes: 12 additions & 1 deletion pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 """
Expand Down
18 changes: 7 additions & 11 deletions pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/test_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
14 changes: 14 additions & 0 deletions pandas/tests/test_indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down