Skip to content
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

API: remove the copy kw from .xs to prevent an expectation of a view (which may not be possible) #6919

Merged
merged 1 commit into from
Apr 22, 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
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
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