BUG: unexpected assign by a single-element list (GH19474) #20732
Conversation
pandas/tests/indexing/test_loc.py
Outdated
| @@ -752,3 +753,21 @@ def convert_nested_indexer(indexer_type, keys): | |||
| index=pd.MultiIndex.from_product(keys)) | |||
|
|
|||
| tm.assert_series_equal(result, expected) | |||
|
|
|||
| @pytest.mark.parametrize( | |||
| 'indexer,value', itertools.product( | |||
There was a problem hiding this comment.
Can you use two instances of parametrize instead of itertools.product?
@pytest.mark.parametrize('indexer', [...])
@pytest.mark.parametrize('value', [...])
def test_loc_setitem_with_scalar_index(self, indexer, value):
pandas/tests/indexing/test_loc.py
Outdated
| df.loc[0, indexer] = value | ||
| result = df.loc[0, 'A'] | ||
|
|
||
| if not is_scalar(result) and result == 'Z': |
There was a problem hiding this comment.
I think it'd be more direct to just do assert result == 'Z'.
Usually in tests you just assert what you want the result to be, instead of catching what you don't want and writing an informative error message like you would in user facing code. If the test does fail, pytest will automatically give details similar to the message you wrote.
There was a problem hiding this comment.
Thank you for comments @jschendel
I rewrote codes, but kept is_scalar(result). Without it, if result is np.array(['Z']) , we cannot catch error because of broadcasting.
BTW, I should have written the conditional statement as not is_scalar(result) and not result == 'Z'. Sorry.
Codecov Report
@@ Coverage Diff @@
## master #20732 +/- ##
=========================================
Coverage ? 91.84%
=========================================
Files ? 153
Lines ? 49295
Branches ? 0
=========================================
Hits ? 45276
Misses ? 4019
Partials ? 0
Continue to review full report at Codecov.
|
doc/source/whatsnew/v0.23.0.txt
Outdated
| @@ -1111,6 +1111,7 @@ Indexing | |||
| - Bug in :meth:`DataFrame.first_valid_index` and :meth:`DataFrame.last_valid_index` in presence of entire rows of NaNs in the middle of values (:issue:`20499`). | |||
| - Bug in :class:`IntervalIndex` where some indexing operations were not supported for overlapping or non-monotonic ``uint64`` data (:issue:`20636`) | |||
| - Bug in ``Series.is_unique`` where extraneous output in stderr is shown if Series contains objects with ``__ne__`` defined (:issue:`20661`) | |||
| - Bug in ``.loc`` where assigned value to ``DataFrame`` is unexpectedly listed (:issue:`19474`) | |||
There was a problem hiding this comment.
Bug in .loc assignment with a single-element list-like incorrectly assigns as a list.
pandas/core/indexing.py
Outdated
| @@ -532,7 +532,8 @@ def setter(item, v): | |||
|
|
|||
| def can_do_equal_len(): | |||
| """ return True if we have an equal len settable """ | |||
| if not len(labels) == 1 or not np.iterable(value): | |||
| if not len(labels) == 1 or not np.iterable(value) \ | |||
There was a problem hiding this comment.
use parens rather than \ to break the line
pandas/tests/indexing/test_loc.py
Outdated
| @@ -752,3 +753,18 @@ def convert_nested_indexer(indexer_type, keys): | |||
| index=pd.MultiIndex.from_product(keys)) | |||
|
|
|||
| tm.assert_series_equal(result, expected) | |||
|
|
|||
There was a problem hiding this comment.
can you move to the end of the tests that start with test_loc_setitem_
pandas/tests/indexing/test_loc.py
Outdated
| @@ -11,6 +11,7 @@ | |||
| from pandas import Series, DataFrame, Timestamp, date_range, MultiIndex, Index | |||
| from pandas.util import testing as tm | |||
| from pandas.tests.indexing.common import Base | |||
| from pandas.core.dtypes.common import is_scalar | |||
There was a problem hiding this comment.
use from pandas.api.types.is_scalar (its the same, just user facing)
jreback
added
Bug
Indexing
pandas/tests/indexing/test_loc.py
Outdated
|
|
||
| @pytest.mark.parametrize( | ||
| 'indexer', [['A'], slice(None, 'A', None), np.array(['A'])]) | ||
| @pytest.mark.parametrize( |
There was a problem hiding this comment.
can you also test this with .iloc
There was a problem hiding this comment.
Updated. Please review.
|
thanks @kittoku keep em coming! |
git diff master --name-only -- "*.py" | flake8I thought it is a straightforward way to evaluate all of
df.loc[0, ['A']] = ['X'],df.loc[0, ['A', 'B']] = ['X', 'Y'], ...(more-columns case) the same way inelseblock at line 590 of pandas/core/indexing.py.Please let me know bad or ungrammatical points.