Skip to content

DEPR/API: disallow lists within list for set_index #24697

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

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
bf0eee0
DEPR/API: disallow lists within list for set_index
h-vetinari Jan 10, 2019
ed0de1f
Add deprecation and whatsnew
h-vetinari Jan 10, 2019
dc274e3
restore test for list-of-scalars interpreted as keys
h-vetinari Jan 10, 2019
623fc9a
Small doc fixes
h-vetinari Jan 10, 2019
5f6e303
Improve docstring; small fixes
h-vetinari Jan 10, 2019
13d4e40
Merge remote-tracking branch 'upstream/master' into depr_LL_set_index
h-vetinari Jan 10, 2019
813b4fc
Remove last mention of "list-like"
h-vetinari Jan 10, 2019
4c130ee
rephrase "illegal"
h-vetinari Jan 10, 2019
8731834
Merge remote-tracking branch 'upstream/master' into depr_LL_set_index
h-vetinari Jan 10, 2019
29fbc6a
Merge remote-tracking branch 'upstream/master' into depr_LL_set_index
h-vetinari Jan 14, 2019
cc04a64
Merge remote-tracking branch 'upstream/master' into depr_LL_set_index
h-vetinari Jan 14, 2019
e1d999b
Improve warning message (review TomAugspurger)
h-vetinari Jan 14, 2019
726ef1c
typo
h-vetinari Jan 14, 2019
0e1f709
Merge remote-tracking branch 'upstream/master' into depr_LL_set_index
h-vetinari Jan 16, 2019
b0b326f
Tuples always considered keys; KeyError, not ValueError if missing
h-vetinari Jan 16, 2019
6cbcc47
Merge remote-tracking branch 'upstream/master' into depr_LL_set_index
h-vetinari Jan 20, 2019
c881aaa
Merge remote-tracking branch 'upstream/master' into depr_LL_set_index
h-vetinari Feb 24, 2019
0214801
Actually commit fix for conflict, duh
h-vetinari Feb 24, 2019
61c511d
Move whatsnew to 0.25
h-vetinari Feb 24, 2019
7381245
Merge remote-tracking branch 'upstream/master' into depr_LL_set_index
h-vetinari Mar 1, 2019
5fa544c
Add deprecation-section (review jreback)
h-vetinari Mar 1, 2019
a016bf0
Merge remote-tracking branch 'upstream/master' into depr_LL_set_index
h-vetinari Mar 3, 2019
0c65876
Fix doc fails
h-vetinari Mar 3, 2019
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
Prev Previous commit
Next Next commit
Add deprecation and whatsnew
  • Loading branch information
h-vetinari committed Jan 10, 2019
commit ed0de1f3756e2f90f8ba0d32c1223375bd9fe9a3
3 changes: 2 additions & 1 deletion doc/source/whatsnew/v0.24.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1251,7 +1251,7 @@ Other API Changes
- :class:`pandas.io.formats.style.Styler` supports a ``number-format`` property when using :meth:`~pandas.io.formats.style.Styler.to_excel` (:issue:`22015`)
- :meth:`DataFrame.corr` and :meth:`Series.corr` now raise a ``ValueError`` along with a helpful error message instead of a ``KeyError`` when supplied with an invalid method (:issue:`22298`)
- :meth:`shift` will now always return a copy, instead of the previous behaviour of returning self when shifting by 0 (:issue:`22397`)
- :meth:`DataFrame.set_index` now allows all one-dimensional list-likes, raises a ``TypeError`` for incorrect types,
- :meth:`DataFrame.set_index` now gives a better (and less frequent) KeyError, and raises a ``ValueError`` for incorrect types,
has an improved ``KeyError`` message, and will not fail on duplicate column names with ``drop=True``. (:issue:`22484`)
- Slicing a single row of a DataFrame with multiple ExtensionArrays of the same type now preserves the dtype, rather than coercing to object (:issue:`22784`)
- :class:`DateOffset` attribute `_cacheable` and method `_should_cache` have been removed (:issue:`23118`)
Expand Down Expand Up @@ -1309,6 +1309,7 @@ Deprecations
- In :meth:`Series.where` with Categorical data, providing an ``other`` that is not present in the categories is deprecated. Convert the categorical to a different dtype or add the ``other`` to the categories first (:issue:`24077`).
- :meth:`Series.clip_lower`, :meth:`Series.clip_upper`, :meth:`DataFrame.clip_lower` and :meth:`DataFrame.clip_upper` are deprecated and will be removed in a future version. Use ``Series.clip(lower=threshold)``, ``Series.clip(upper=threshold)`` and the equivalent ``DataFrame`` methods (:issue:`24203`)
- :meth:`Series.nonzero` is deprecated and will be removed in a future version (:issue:`18262`)
- :meth:`DataFrame.set_index` has deprecated using lists of values *within* lists. It remains possible to pass array-likes, both directly and within a list.

.. _whatsnew_0240.deprecations.datetimelike_int_ops:

Expand Down
11 changes: 9 additions & 2 deletions pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -4139,20 +4139,27 @@ def set_index(self, keys, drop=True, append=False, inplace=False,
raise ValueError(err_msg)

missing = []
depr_warn = False
for col in keys:
if (is_scalar(col) or isinstance(col, tuple)) and col in self:
# if col is a valid column key, everything is fine
continue
elif is_scalar(col) and col not in self:
# tuples that are not keys will be are excluded here;
# will be considered list-like, not missing
# tuples that are not keys are not considered missing,
# but as an illegal list-like
missing.append(col)
elif isinstance(col, list):
depr_warn = True
elif (not isinstance(col, (ABCIndexClass, ABCSeries, np.ndarray))
or getattr(col, 'ndim', 1) > 1):
raise ValueError(err_msg)

if missing:
raise KeyError('{}'.format(missing))
if depr_warn:
msg = ('passing lists within a list to the parameter "keys" is '
'deprecated and will be removed in a future version.')
warnings.warn(msg, FutureWarning, stacklevel=2)

if inplace:
frame = self
Expand Down
36 changes: 27 additions & 9 deletions pandas/tests/frame/test_alter_axes.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def test_set_index_pass_single_array(self, frame_of_index_cols,

# MultiIndex constructor does not work directly on Series -> lambda
# also test index name if append=True (name is duplicate here for A & B)
@pytest.mark.parametrize('box', [Series, Index, np.array,
@pytest.mark.parametrize('box', [Series, Index, np.array, list,
lambda x: MultiIndex.from_arrays([x])])
@pytest.mark.parametrize('append, index_name',
[(True, None), (True, 'A'), (True, 'B'),
Expand All @@ -156,9 +156,13 @@ def test_set_index_pass_arrays(self, frame_of_index_cols,

keys = ['A', box(df['B'])]
# np.array "forgets" the name of B
names = ['A', None if box in [np.array] else 'B']
names = ['A', None if box in [list, np.array] else 'B']

result = df.set_index(keys, drop=drop, append=append)
if box == list:
with tm.assert_produces_warning(FutureWarning):
result = df.set_index(keys, drop=drop, append=append)
else:
result = df.set_index(keys, drop=drop, append=append)

# only valid column keys are dropped
# since B is always passed as array above, only A is dropped, if at all
Expand All @@ -171,10 +175,10 @@ def test_set_index_pass_arrays(self, frame_of_index_cols,
# MultiIndex constructor does not work directly on Series -> lambda
# We also emulate a "constructor" for the label -> lambda
# also test index name if append=True (name is duplicate here for A)
@pytest.mark.parametrize('box2', [Series, Index, np.array,
@pytest.mark.parametrize('box2', [Series, Index, np.array, list,
lambda x: MultiIndex.from_arrays([x]),
lambda x: x.name])
@pytest.mark.parametrize('box1', [Series, Index, np.array,
@pytest.mark.parametrize('box1', [Series, Index, np.array, list,
lambda x: MultiIndex.from_arrays([x]),
lambda x: x.name])
@pytest.mark.parametrize('append, index_name', [(True, None),
Expand All @@ -186,17 +190,31 @@ def test_set_index_pass_arrays_duplicate(self, frame_of_index_cols, drop,
df.index.name = index_name

keys = [box1(df['A']), box2(df['A'])]
result = df.set_index(keys, drop=drop, append=append)

if box1 == list or box2 == list:
with tm.assert_produces_warning(FutureWarning):
result = df.set_index(keys, drop=drop, append=append)
else:
result = df.set_index(keys, drop=drop, append=append)

# need to adapt first drop for case that both keys are 'A' --
# cannot drop the same column twice;
# use "is" because == would give ambiguous Boolean error for containers
first_drop = False if (keys[0] is 'A' and keys[1] is 'A') else drop

# to test against already-tested behaviour, we add sequentially,
# hence second append always True
expected = df.set_index(keys[0], drop=first_drop, append=append)
expected = expected.set_index(keys[1], drop=drop, append=True)
# hence second append always True; must wrap keys in list, otherwise
# box = list would be interpreted as keys
if box1 == list or box2 == list:
with tm.assert_produces_warning(FutureWarning):
expected = df.set_index([keys[0]], drop=first_drop,
append=append)
expected = expected.set_index([keys[1]], drop=drop,
append=True)
else:
expected = df.set_index([keys[0]], drop=first_drop, append=append)
expected = expected.set_index([keys[1]], drop=drop, append=True)

tm.assert_frame_equal(result, expected)

@pytest.mark.parametrize('append', [True, False])
Expand Down