Skip to content

Commit

Permalink
API: remove the copy kw from .xs to prevent accidental / confusing se…
Browse files Browse the repository at this point in the history
…tting (GH6894)
  • Loading branch information
jreback committed Apr 22, 2014
1 parent 95090fd commit c81712c
Show file tree
Hide file tree
Showing 12 changed files with 129 additions and 100 deletions.
2 changes: 1 addition & 1 deletion doc/source/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ SQL

.. autosummary::
:toctree: generated/

read_sql_table
read_sql_query
read_sql
Expand Down
4 changes: 4 additions & 0 deletions doc/source/release.rst
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,10 @@ API Changes
- ``.quantile`` on a ``datetime[ns]`` series now returns ``Timestamp`` instead
of ``np.datetime64`` objects (:issue:`6810`)

- Remove the ``copy`` keyword from ``DataFrame.xs``,``Panel.major_xs``,``Panel.minor_xs``. A view will be
returned if possible, otherwise a copy will be made. Previously the user could think that ``copy=False`` would
ALWAYS return a view. (:issue:`6894`)

Deprecations
~~~~~~~~~~~~

Expand Down
4 changes: 4 additions & 0 deletions doc/source/v0.14.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,10 @@ API changes
(and numpy defaults)
- add ``inplace`` keyword to ``Series.order/sort`` to make them inverses (:issue:`6859`)

- Remove the ``copy`` keyword from ``DataFrame.xs``,``Panel.major_xs``,``Panel.minor_xs``. A view will be
returned if possible, otherwise a copy will be made. Previously the user could think that ``copy=False`` would
ALWAYS return a view. (:issue:`6894`)

.. _whatsnew_0140.sql:

SQL
Expand Down
7 changes: 5 additions & 2 deletions pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -1564,7 +1564,7 @@ def irow(self, i, copy=False):
def icol(self, i):
return self._ixs(i, axis=1)

def _ixs(self, i, axis=0, copy=False):
def _ixs(self, i, axis=0):
"""
i : int, slice, or sequence of integers
axis : int
Expand All @@ -1588,7 +1588,10 @@ def _ixs(self, i, axis=0, copy=False):
result = self.take(i, axis=axis)
copy=True
else:
new_values, copy = self._data.fast_xs(i, copy=copy)
new_values = self._data.fast_xs(i)

# if we are a copy, mark as such
copy = isinstance(new_values,np.ndarray) and new_values.base is None
result = Series(new_values, index=self.columns,
name=self.index[i], dtype=new_values.dtype)
result._set_is_copy(self, copy=copy)
Expand Down
33 changes: 15 additions & 18 deletions pandas/core/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -1240,7 +1240,7 @@ def take(self, indices, axis=0, convert=True, is_copy=True):

return result

def xs(self, key, axis=0, level=None, copy=True, drop_level=True):
def xs(self, key, axis=0, level=None, copy=None, drop_level=True):
"""
Returns a cross-section (row(s) or column(s)) from the Series/DataFrame.
Defaults to cross-section on the rows (axis=0).
Expand All @@ -1254,7 +1254,7 @@ def xs(self, key, axis=0, level=None, copy=True, drop_level=True):
level : object, defaults to first n levels (n=1 or len(key))
In case of a key partially contained in a MultiIndex, indicate
which levels are used. Levels can be referred by label or position.
copy : boolean, default True
copy : boolean [deprecated]
Whether to make a copy of the data
drop_level : boolean, default True
If False, returns object with same levels as self.
Expand All @@ -1276,14 +1276,6 @@ def xs(self, key, axis=0, level=None, copy=True, drop_level=True):
b 9
c 3
Name: C
>>> s = df.xs('a', copy=False)
>>> s['A'] = 100
>>> df
A B C
a 100 5 2
b 4 0 9
c 9 7 3
>>> df
A B C D
Expand All @@ -1310,16 +1302,24 @@ def xs(self, key, axis=0, level=None, copy=True, drop_level=True):
-------
xs : Series or DataFrame
Notes
-----
xs is only for getting, not setting values.
MultiIndex Slicers is a generic way to get/set values on any level or levels
it is a superset of xs functionality, see :ref:`MultiIndex Slicers <indexing.mi_slicers>`
"""
if copy is not None:
warnings.warn("copy keyword is deprecated, "
"default is to return a copy or a view if possible")

axis = self._get_axis_number(axis)
labels = self._get_axis(axis)
if level is not None:
loc, new_ax = labels.get_loc_level(key, level=level,
drop_level=drop_level)

if not copy and not isinstance(loc, slice):
raise ValueError('Cannot retrieve view (copy=False)')

# convert to a label indexer if needed
if isinstance(loc, slice):
lev_num = labels._get_level_number(level)
Expand All @@ -1336,10 +1336,7 @@ def xs(self, key, axis=0, level=None, copy=True, drop_level=True):
return result

if axis == 1:
data = self[key]
if copy:
data = data.copy()
return data
return self[key]

self._consolidate_inplace()

Expand All @@ -1362,7 +1359,7 @@ def xs(self, key, axis=0, level=None, copy=True, drop_level=True):

if np.isscalar(loc):
from pandas import Series
new_values, copy = self._data.fast_xs(loc, copy=copy)
new_values = self._data.fast_xs(loc)

# may need to box a datelike-scalar
#
Expand Down
7 changes: 2 additions & 5 deletions pandas/core/indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,14 @@ def _get_label(self, label, axis=0):
# but will fail when the index is not present
# see GH5667
try:
return self.obj._xs(label, axis=axis, copy=False)
return self.obj._xs(label, axis=axis)
except:
return self.obj[label]
elif (isinstance(label, tuple) and
isinstance(label[axis], slice)):
raise IndexingError('no slices here, handle elsewhere')

try:
return self.obj._xs(label, axis=axis, copy=False)
except Exception:
return self.obj._xs(label, axis=axis, copy=True)
return self.obj._xs(label, axis=axis)

def _get_loc(self, key, axis=0):
return self.obj._ixs(key, axis=axis)
Expand Down
24 changes: 10 additions & 14 deletions pandas/core/internals.py
Original file line number Diff line number Diff line change
Expand Up @@ -2861,9 +2861,7 @@ def xs(self, key, axis=1, copy=True, takeable=False):

new_blocks = []
if len(self.blocks) > 1:
if not copy:
raise Exception('cannot get view of mixed-type or '
'non-consolidated DataFrame')
# we must copy here as we are mixed type
for blk in self.blocks:
newb = make_block(blk.values[slicer],
blk.items,
Expand All @@ -2884,18 +2882,16 @@ def xs(self, key, axis=1, copy=True, takeable=False):

return self.__class__(new_blocks, new_axes)

def fast_xs(self, loc, copy=False):
def fast_xs(self, loc):
"""
get a cross sectional for a given location in the
items ; handle dups
return the result and a flag if a copy was actually made
return the result, is *could* be a view in the case of a

This comment has been minimized.

Copy link
@samueljohn

samueljohn Nov 17, 2015

"it" instead of "is"?

single block
"""
if len(self.blocks) == 1:
result = self.blocks[0].values[:, loc]
if copy:
result = result.copy()
return result, copy
return self.blocks[0].values[:, loc]

items = self.items

Expand All @@ -2904,7 +2900,7 @@ def fast_xs(self, loc, copy=False):
result = self._interleave(items)
if self.ndim == 2:
result = result.T
return result[loc], True
return result[loc]

# unique
dtype = _interleaved_dtype(self.blocks)
Expand All @@ -2915,7 +2911,7 @@ def fast_xs(self, loc, copy=False):
i = items.get_loc(item)
result[i] = blk._try_coerce_result(blk.iget((j, loc)))

return result, True
return result

def consolidate(self):
"""
Expand Down Expand Up @@ -3829,12 +3825,12 @@ def _consolidate_check(self):
def _consolidate_inplace(self):
pass

def fast_xs(self, loc, copy=False):
def fast_xs(self, loc):
"""
fast path for getting a cross-section
return a view of the data
"""
result = self._block.values[loc]
return result, False
return self._block.values[loc]

def construction_error(tot_items, block_shape, axes, e=None):
""" raise a helpful message about our construction """
Expand Down
70 changes: 53 additions & 17 deletions pandas/core/panel.py
Original file line number Diff line number Diff line change
Expand Up @@ -688,43 +688,67 @@ def _combine_panel(self, other, func):

return self._constructor(result_values, items, major, minor)

def major_xs(self, key, copy=True):
def major_xs(self, key, copy=None):
"""
Return slice of panel along major axis
Parameters
----------
key : object
Major axis label
copy : boolean, default True
Copy data
copy : boolean [deprecated]
Whether to make a copy of the data
Returns
-------
y : DataFrame
index -> minor axis, columns -> items
Notes
-----
major_xs is only for getting, not setting values.
MultiIndex Slicers is a generic way to get/set values on any level or levels
it is a superset of major_xs functionality, see :ref:`MultiIndex Slicers <indexing.mi_slicers>`
"""
return self.xs(key, axis=self._AXIS_LEN - 2, copy=copy)
if copy is not None:
warnings.warn("copy keyword is deprecated, "
"default is to return a copy or a view if possible")

def minor_xs(self, key, copy=True):
return self.xs(key, axis=self._AXIS_LEN - 2)

def minor_xs(self, key, copy=None):
"""
Return slice of panel along minor axis
Parameters
----------
key : object
Minor axis label
copy : boolean, default True
Copy data
copy : boolean [deprecated]
Whether to make a copy of the data
Returns
-------
y : DataFrame
index -> major axis, columns -> items
Notes
-----
minor_xs is only for getting, not setting values.
MultiIndex Slicers is a generic way to get/set values on any level or levels
it is a superset of minor_xs functionality, see :ref:`MultiIndex Slicers <indexing.mi_slicers>`
"""
return self.xs(key, axis=self._AXIS_LEN - 1, copy=copy)
if copy is not None:
warnings.warn("copy keyword is deprecated, "
"default is to return a copy or a view if possible")

return self.xs(key, axis=self._AXIS_LEN - 1)

def xs(self, key, axis=1, copy=True):
def xs(self, key, axis=1, copy=None):
"""
Return slice of panel along selected axis
Expand All @@ -733,24 +757,36 @@ def xs(self, key, axis=1, copy=True):
key : object
Label
axis : {'items', 'major', 'minor}, default 1/'major'
copy : boolean, default True
Copy data
copy : boolean [deprecated]
Whether to make a copy of the data
Returns
-------
y : ndim(self)-1
Notes
-----
xs is only for getting, not setting values.
MultiIndex Slicers is a generic way to get/set values on any level or levels
it is a superset of xs functionality, see :ref:`MultiIndex Slicers <indexing.mi_slicers>`
"""
if copy is not None:
warnings.warn("copy keyword is deprecated, "
"default is to return a copy or a view if possible")

axis = self._get_axis_number(axis)
if axis == 0:
data = self[key]
if copy:
data = data.copy()
return data
return self[key]

self._consolidate_inplace()
axis_number = self._get_axis_number(axis)
new_data = self._data.xs(key, axis=axis_number, copy=copy)
return self._construct_return_type(new_data)
new_data = self._data.xs(key, axis=axis_number, copy=False)
result = self._construct_return_type(new_data)
copy = new_data.is_mixed_type
result._set_is_copy(self, copy=copy)
return result

_xs = xs

Expand Down
29 changes: 8 additions & 21 deletions pandas/tests/test_frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -8371,12 +8371,8 @@ def test_xs(self):
expected = self.frame['A']
assert_series_equal(series, expected)

# no view by default
series[:] = 5
self.assert_((expected != 5).all())

# view
series = self.frame.xs('A', axis=1, copy=False)
# view is returned if possible
series = self.frame.xs('A', axis=1)
series[:] = 5
self.assert_((expected == 5).all())

Expand Down Expand Up @@ -11888,25 +11884,16 @@ def test_boolean_set_uncons(self):
assert_almost_equal(expected, self.frame.values)

def test_xs_view(self):
"""
in 0.14 this will return a view if possible
a copy otherwise, but this is numpy dependent
"""

dm = DataFrame(np.arange(20.).reshape(4, 5),
index=lrange(4), columns=lrange(5))

dm.xs(2, copy=False)[:] = 5
self.assert_((dm.xs(2) == 5).all())

dm.xs(2)[:] = 10
self.assert_((dm.xs(2) == 5).all())

# prior to chained assignment (GH5390)
# this would raise, but now just returns a copy (and sets is_copy)
# TODO (?): deal with mixed-type fiasco?
# with assertRaisesRegexp(TypeError, 'cannot get view of mixed-type'):
# self.mixed_frame.xs(self.mixed_frame.index[2], copy=False)

# unconsolidated
dm['foo'] = 6.
dm.xs(3, copy=False)[:] = 10
self.assert_((dm.xs(3) == 10).all())
self.assert_((dm.xs(2) == 10).all())

def test_boolean_indexing(self):
idx = lrange(3)
Expand Down
Loading

0 comments on commit c81712c

Please sign in to comment.