-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: Inserting array of same size with Series.loc raises ValueError #38266
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
BUG: Inserting array of same size with Series.loc raises ValueError #38266
Conversation
cc @phofl |
pandas/tests/indexing/test_iloc.py
Outdated
# GH37748 | ||
ser = Series(0, index=range(5), dtype="object") | ||
|
||
expected = np.zeros(size) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you get the same bug with a length size
python list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
doc/source/whatsnew/v1.2.0.rst
Outdated
@@ -677,6 +677,7 @@ Indexing | |||
- Bug in :meth:`DataFrame.loc` and :meth:`DataFrame.__getitem__` raising ``KeyError`` when columns were :class:`MultiIndex` with only one level (:issue:`29749`) | |||
- Bug in :meth:`Series.__getitem__` and :meth:`DataFrame.__getitem__` raising blank ``KeyError`` without missing keys for :class:`IntervalIndex` (:issue:`27365`) | |||
- Bug in setting a new label on a :class:`DataFrame` or :class:`Series` with a :class:`CategoricalIndex` incorrectly raising ``TypeError`` when the new label is not among the index's categories (:issue:`38098`) | |||
- Bug in :meth:`Series.loc` and :meth:`Series.iloc` raising ``ValueError`` when inserting an array in a ``object`` Series of equal length (:issue:`37748`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
array seems vague. Do you mean NumPy array, list, ExtensionArray (or selection of those)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all of those i guess :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, np.array, list, tuple do cause the raise. From your example, ExtensionArray don't but also behave unexpectedly.
Also does this solve #38271 |
It seems so:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay great can you add tests for the list, ExtensionArray cases and update whatsnew
doc/source/whatsnew/v1.2.0.rst
Outdated
@@ -677,6 +677,7 @@ Indexing | |||
- Bug in :meth:`DataFrame.loc` and :meth:`DataFrame.__getitem__` raising ``KeyError`` when columns were :class:`MultiIndex` with only one level (:issue:`29749`) | |||
- Bug in :meth:`Series.__getitem__` and :meth:`DataFrame.__getitem__` raising blank ``KeyError`` without missing keys for :class:`IntervalIndex` (:issue:`27365`) | |||
- Bug in setting a new label on a :class:`DataFrame` or :class:`Series` with a :class:`CategoricalIndex` incorrectly raising ``TypeError`` when the new label is not among the index's categories (:issue:`38098`) | |||
- Bug in :meth:`Series.loc` and :meth:`Series.iloc` raising ``ValueError`` when inserting an array in a ``object`` Series of equal length (:issue:`37748`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all of those i guess :)
pandas/_testing.py
Outdated
def assert_python_equal(left, right): | ||
""" | ||
Check left and right are equal w.r.t the ``==`` operator. | ||
|
||
Parameters | ||
---------- | ||
left : object | ||
right : object | ||
""" | ||
assert left == right |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I'm guessing this is too much, but I failed to find helper funcs to assert list/tuple equality.
I'll welcome advice on how to write the tests :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah don't do this, just do assert left == right
pytest already handles this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
pandas/tests/indexing/test_loc.py
Outdated
|
||
ser.loc["a"] = expected | ||
result = ser[0] | ||
assert_fn(result, expected) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason for not checking the whole Series? This would also avoid your helper function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point, thanks
pandas/_testing.py
Outdated
def assert_python_equal(left, right): | ||
""" | ||
Check left and right are equal w.r.t the ``==`` operator. | ||
|
||
Parameters | ||
---------- | ||
left : object | ||
right : object | ||
""" | ||
assert left == right |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah don't do this, just do assert left == right
pytest already handles this
doc/source/whatsnew/v1.2.0.rst
Outdated
@@ -677,6 +677,8 @@ Indexing | |||
- Bug in :meth:`DataFrame.loc` and :meth:`DataFrame.__getitem__` raising ``KeyError`` when columns were :class:`MultiIndex` with only one level (:issue:`29749`) | |||
- Bug in :meth:`Series.__getitem__` and :meth:`DataFrame.__getitem__` raising blank ``KeyError`` without missing keys for :class:`IntervalIndex` (:issue:`27365`) | |||
- Bug in setting a new label on a :class:`DataFrame` or :class:`Series` with a :class:`CategoricalIndex` incorrectly raising ``TypeError`` when the new label is not among the index's categories (:issue:`38098`) | |||
- Bug in :meth:`Series.loc` and :meth:`Series.iloc` raising ``ValueError`` when inserting a listlike ``np.array``, ``list`` or ``tuple`` in an ``object`` Series of equal length (:issue:`37748`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you list 3 issues in the top of PR, what's missing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls add the issue number to the note for #37748 then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
cc @phofl and @jbrockmendel if any comments. |
lgtm otherwise |
thanks @ma3da |
closes #37748
closes #37486
closes #38271
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff