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.

Sign up for GitHub

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

Jump to bottom

BUG: loc setitem with missing integer slices does not raise #30921

Closed
wants to merge 1 commit into from
Closed

BUG: loc setitem with missing integer slices does not raise #30921

wants to merge 1 commit into from

Conversation

fujiaxiang
Copy link
Member

@fujiaxiang
Copy link
Member Author

There are two reasons for updating an old test case test_loc_setitem_empty_append_raises in test_loc.py:

  1. To raise the same error as .loc.__getitem__ when give slice does not exist

  2. The current error says cannot copy sequence with size 2 to array axis with dimension 0. This is not in line with what the user is trying to do: to add 2 new rows or columns. The error should be a TypeError because we don't current support (not yet implemented) using integer slice to add new rows / columns.

Hope that sounds convincing.
If not, let me know.

@fujiaxiang
Copy link
Member Author

This change has a huge impact on existing test cases. Let me investigate further.

@gfyoung gfyoung added Bug Indexing Related to indexing on series/frames, not to indexes themselves labels Jan 15, 2020
gfyoung
gfyoung reviewed Jan 15, 2020
View reviewed changes
doc/source/whatsnew/v1.0.0.rst
@@ -1036,6 +1036,7 @@ Indexing
- :meth:`MultiIndex.get_loc` can't find missing values when input includes missing values (:issue:`19132`)
- Bug in :meth:`Series.__setitem__` incorrectly assigning values with boolean indexer when the length of new data matches the number of ``True`` values and new data is not a ``Series`` or an ``np.array`` (:issue:`30567`)
- Bug in indexing with a :class:`PeriodIndex` incorrectly accepting integers representing years, use e.g. ``ser.loc["2007"]`` instead of ``ser.loc[2007]`` (:issue:`30763`)
- Bug in :meth:`DataFrame.loc.__setitem__` and :meth:`Series.loc.__setitem__` not raising error when given integer slice does not exist in index (:issue:`26412`)
Copy link
Member

@gfyoung gfyoung Jan 15, 2020 •
edited

Choose a reason for hiding this comment

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

Suggested change
- Bug in :meth:`DataFrame.loc.__setitem__` and :meth:`Series.loc.__setitem__` not raising error when given integer slice does not exist in index (:issue:`26412`)
- Bug in :meth:`DataFrame.loc.__setitem__` and :meth:`Series.loc.__setitem__` not raising error when given integer slice that do not exist in the index (:issue:`26412`)

gfyoung
gfyoung reviewed Jan 15, 2020
View reviewed changes
pandas/tests/indexing/test_loc.py
# GH 26412
df = DataFrame({"col1": [1, 2, 3]}, index=["a", "b", "c"])
msg = "cannot do slice indexing on .* with these indexers"
with pytest.raises(TypeError, match=msg):
Copy link
Member

Choose a reason for hiding this comment

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

Newline above this one

jbrockmendel
jbrockmendel reviewed Feb 11, 2020
View reviewed changes
pandas/core/indexes/base.py
@@ -2922,6 +2922,9 @@ def is_int(v):
self._validate_indexer("slice", key.step, kind),
)

if kind == "loc" and not is_null_slicer:
return self.slice_indexer(key.start, key.stop, key.step, kind=kind)

Copy link
Member

Choose a reason for hiding this comment

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

is any of the validation on L2931-2948 relevant? can some or all of it be removed?

This likely affects some tests, see how this is handled in #31840.

Copy link
Member Author

Choose a reason for hiding this comment

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

hey sorry I haven't had time to work on this.
will close this PR in favor of #31840.

@fujiaxiang
Copy link
Member Author

closing this PR in favor of #31840.

@fujiaxiang fujiaxiang closed this Feb 28, 2020
@fujiaxiang fujiaxiang deleted the loc_setitem_with_integer_slices_does_not_raise branch March 20, 2021 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbrockmendel jbrockmendel jbrockmendel left review comments

@gfyoung gfyoung gfyoung left review comments

Assignees
No one assigned
Labels
Bug Indexing Related to indexing on series/frames, not to indexes themselves
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Using .loc.__setitem__ with integer slices does not raise
3 participants
@fujiaxiang @jbrockmendel @gfyoung