REF: move reshaping of array for setitem from DataFrame into BlockManager internals #39722
Conversation
jorisvandenbossche
added
Refactor
|
|
||
| class TestReshaping(BaseNumPyTests, base.BaseReshapingTests): | ||
| @skip_nested | ||
| @pytest.mark.skip(reason="Incorrect expected.") |
There was a problem hiding this comment.
I don't fully understand how testing with the PandasArrays works, but, within the merge implementation, we set a column in the resulting dataframe (the column with the key values) using an Index.
And doing an extract_array on an Int64Index gives a PandasDtype("int64") column when running those tests. And because in this PR I changed it such that the values being set are not converted to a 1D array before ending up in BlockManager.iset, the extension dtype is now preserved. And so the expected result from the base test is wrong
There was a problem hiding this comment.
this turns out to be a PITA in a bunch of places. could just monkeypatch extract_array to extract PandasArray too
| "'self.flags.allows_duplicate_labels' is False." | ||
| ) | ||
| value = self._sanitize_column(value) | ||
| value = _maybe_atleast_2d(value) |
There was a problem hiding this comment.
i think this removed all the usages of maybe_atleast_2d, so can remove the function
There was a problem hiding this comment.
Indeed, will remove that in one of my next PRs
| if value.ndim == 2: | ||
| value = value.T | ||
|
|
||
| if value.ndim == self.ndim - 1: |
There was a problem hiding this comment.
indeed, will also include in a next PR
There was a problem hiding this comment.
Actually, it needs to be an if, because this case needs to end up in the final else to define the value_getitem function
| if value.ndim == 2: | ||
| value = value.T | ||
|
|
||
| if value.ndim == self.ndim - 1 and not is_extension_array_dtype(value.dtype): |
|
sorry for so quick @jbrockmendel will wait in future |
|
Yeah, I explicitly marked Brock for review ;) |
3 participants
I think ideally the DataFrame does not need to be aware of how the underlying manager stores the data (as 2D, transposed or not), so moving the logic of ensuring 2D/transposed values from the DataFrame set_item-related method into
BlockManager.iset.This will help for the ArrayManager, so we don't have to re-reshape there.