BUG: df.loc setitem-with-expansion with duplicate index #40096
Conversation
|
cc @phofl |
pandas/tests/indexing/test_loc.py
Outdated
|
|
||
| exp_index = index.insert(len(index), key) | ||
| if isinstance(index, MultiIndex): | ||
| exp_index = index.insert(len(index), key) |
There was a problem hiding this comment.
I think this is not needed?
LGTM otherwise
| df.loc["d", "A"] = 10 | ||
| with pytest.raises(TypeError, match=msg): | ||
| df.loc["d", "C"] = 10 | ||
| # Setting-with-expansion with a new key "d" that is not among caegories |
There was a problem hiding this comment.
might be worth splitting this test (e.g. errors to a new tests) and even these cases
| expected = DataFrame(exp_data, index=exp_index, columns=[0]) | ||
|
|
||
| # Add new row, but no new columns | ||
| df = orig.copy() |
There was a problem hiding this comment.
same might be worth splitting this up a bit
pandas/core/indexing.py
Outdated
| @@ -1641,7 +1641,12 @@ def _setitem_with_indexer(self, indexer, value, name="iloc"): | |||
| # so the object is the same | |||
| index = self.obj._get_axis(i) | |||
| labels = index.insert(len(index), key) | |||
| self.obj._mgr = self.obj.reindex(labels, axis=i)._mgr | |||
| taker = list(range(len(index))) + [-1] | |||
There was a problem hiding this comment.
can you add comments on what is happening here
There was a problem hiding this comment.
prob doesn't make much difference but can you just use np.arange here?
There was a problem hiding this comment.
In [2]: %timeit list(np.arange(10))
1.6 µs ± 15.6 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
In [3]: %timeit list(range(10))
238 ns ± 7.09 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
There was a problem hiding this comment.
- sure that's small sample, is it always really small like this?
- needs to have ensure_int_platform generally (you can directly construct the np array like that)
- we always use np.arange for this so changing patterns is not helpful
There was a problem hiding this comment.
sure that's small sample, is it always really small like this?
In [2]: N = 10**6
In [3]: %timeit list(np.arange(N))
79.6 ms ± 3.4 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)
In [4]: %timeit list(range(N))
32.9 ms ± 1.85 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)
There was a problem hiding this comment.
wait, better idea, never mind.
There was a problem hiding this comment.
N = 10
In [5]: %timeit arr = np.asarray(list(range(N)) + [1], dtype=np.intp)
2.52 µs ± 110 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
In [6]: %timeit arr = np.arange(N + 1); arr[-1] = -1
533 ns ± 11 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
N = 10**6
In [8]: %timeit arr = np.arange(N + 1); arr[-1] = -1
397 µs ± 24 µs per loop (mean ± std. dev. of 7 runs, 1000 loops each)
In [9]: %timeit arr = np.asarray(list(range(N)) + [1], dtype=np.intp)
98.6 ms ± 437 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)
jreback
added
the
Indexing
|
I think we should deprecate indexing expansion, pretty sure we have an issue for this. |
|
updted per comments + greenish |
also CategoricalIndex.insert