Skip to content

BUG: Multiindexed series .at fix #32520

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 6 commits into from
May 25, 2020
Merged
Show file tree
Hide file tree
Changes from 3 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/whatsnew/v1.1.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ Indexing
- Bug in :meth:`Series.__setitem__` with an :class:`IntervalIndex` and a list-like key of integers (:issue:`33473`)
- Bug in :meth:`Series.__getitem__` allowing missing labels with ``np.ndarray``, :class:`Index`, :class:`Series` indexers but not ``list``, these now all raise ``KeyError`` (:issue:`33646`)
- Bug in :meth:`DataFrame.truncate` and :meth:`Series.truncate` where index was assumed to be monotone increasing (:issue:`33756`)
- Indexing with a list of strings representing datetimes failed on :class:`DatetimeIndex` or :class:`PeriodIndex`(:issue:`11278`)
- Bug in :meth:`Series.at` when used with a :class:`MultiIndex` would raise an exception on valid inputs (:issue:`26989`)

Missing
^^^^^^^
Expand Down
8 changes: 7 additions & 1 deletion pandas/core/indexing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2016,10 +2016,10 @@ def __setitem__(self, key, value):

if not isinstance(key, tuple):
key = _tuplify(self.ndim, key)
key = list(self._convert_key(key, is_setter=True))
if len(key) != self.ndim:
raise ValueError("Not enough indexers for scalar access (setting)!")

key = list(self._convert_key(key, is_setter=True))
self.obj._set_value(*key, value=value, takeable=self._takeable)


Expand All @@ -2032,6 +2032,12 @@ def _convert_key(self, key, is_setter: bool = False):
Require they keys to be the same type as the index. (so we don't
fallback)
"""
# GH 26989
# For series, unpacking key needs to result in the label.
# This is already the case for len(key) == 1; e.g. (1,)
if self.ndim == 1 and len(key) > 1:
key = (key,)

# allow arbitrary setting
if is_setter:
return list(key)
Expand Down
62 changes: 62 additions & 0 deletions pandas/tests/indexing/test_scalar.py
Original file line number Diff line number Diff line change
Expand Up @@ -351,3 +351,65 @@ def test_iat_series_with_period_index():
expected = ser[index[0]]
result = ser.iat[0]
assert expected == result


def test_at_with_tuple_index_get():
# GH 26989
# DataFrame.at getter works with Index of tuples
df = DataFrame({"a": [1, 2]}, index=[(1, 2), (3, 4)])
assert df.index.nlevels == 1
assert df.at[(1, 2), "a"] == 1

# Series.at getter works with Index of tuples
series = df["a"]
assert series.index.nlevels == 1
assert series.at[(1, 2)] == 1


def test_at_with_tuple_index_set():
# GH 26989
# DataFrame.at setter works with Index of tuples
df = DataFrame({"a": [1, 2]}, index=[(1, 2), (3, 4)])
assert df.index.nlevels == 1
df.at[(1, 2), "a"] = 2
assert df.at[(1, 2), "a"] == 2

# Series.at setter works with Index of tuples
series = df["a"]
assert series.index.nlevels == 1
series.at[1, 2] = 3
assert series.at[1, 2] == 3


def test_multiindex_at_get():
# GH 26989
# DataFrame.at and DataFrame.loc getter works with MultiIndex
df = DataFrame({"a": [1, 2]}, index=[[1, 2], [3, 4]])
assert df.index.nlevels == 2
assert df.at[(1, 3), "a"] == 1
assert df.loc[(1, 3), "a"] == 1

# Series.at and Series.loc getter works with MultiIndex
series = df["a"]
assert series.index.nlevels == 2
assert series.at[1, 3] == 1
assert series.loc[1, 3] == 1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test for not providing all MultiIndex levels? (so eg series.at[1]).
And also a test with a duplicated label?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And maybe also an equivalent test for a DataFrame (it seems to work, but would be good to ensure we also have an explicit test for that)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure - I have it mostly done, but ran into a question. For the series

series = pd.Series([1, 2, 3], index=[[1, 2, 2], [3, 4, 4]])

i.e.

1  3    1
2  4    2
   4    3
dtype: int64

what should be the output of series.at[1, 3]? I was expecting to just get the scalar 1, but currently I'm getting

1  3    1
dtype: int64

After thinking about this - perhaps what we have currently is the desired output. My reason for this is that the shape doesn't change depending on the input - (1, 3) vs (2, 4).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, that should be a scalar (but that is also the reason that I asked to test it, to be sure this is covered ànd to discuss the semantics we want)


def test_multiindex_at_set():
# GH 26989
# DataFrame.at and DataFrame.loc setter works with MultiIndex
df = DataFrame({"a": [1, 2]}, index=[[1, 2], [3, 4]])
assert df.index.nlevels == 2
df.at[(1, 3), "a"] = 3
assert df.at[(1, 3), "a"] == 3
df.loc[(1, 3), "a"] = 4
assert df.loc[(1, 3), "a"] == 4

# Series.at and Series.loc setter works with MultiIndex
series = df["a"]
assert series.index.nlevels == 2
series.at[1, 3] = 5
assert series.at[1, 3] == 5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the behavior now on master? IOW is this an api change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current behavior on master is to raise ValueError for any of the at lines, behavior with the loc lines is the same as master. I would describe this as a bugfix and not an api change.

series.loc[1, 3] = 6
assert series.loc[1, 3] == 6