BUG: DataFrame.__setitem__ not raising ValueError when rhs is df and has wrong number of columns #39341
Conversation
…has wrong number of columns
phofl
added
the
Indexing
| # align right-hand-side columns if self.columns | ||
| # is multi-index and self[key] is a sub-frame | ||
| if isinstance(self.columns, MultiIndex) and key in self.columns: | ||
| if key in self.columns: | ||
| loc = self.columns.get_loc(key) |
There was a problem hiding this comment.
can you add a comment: # align columns before L3259
pandas/core/frame.py
Outdated
| # align right-hand-side columns if self.columns | ||
| # is multi-index and self[key] is a sub-frame | ||
| if isinstance(self.columns, MultiIndex): | ||
| if isinstance(loc, (slice, Series, np.ndarray, Index)): |
There was a problem hiding this comment.
can combine these conditions
|
thanks @phofl |
| df = DataFrame([[1, 2, 3]], columns=["a", "b", "b"]) | ||
| rhs = DataFrame([[10, 11, 12]], columns=["d", "e", "c"]) | ||
| df[["a", "b"]] = rhs | ||
| expected = DataFrame([[10, 11, 12]], columns=["a", "b", "b"]) |
There was a problem hiding this comment.
is this obvious? is rhs.columns irrelevant?
There was a problem hiding this comment.
What do you mean with obvious?
Yes columns are irrelevant with setitem
| @@ -3223,7 +3223,7 @@ def _setitem_array(self, key, value): | |||
| self._check_setitem_copy() | |||
| self.iloc[indexer] = value | |||
| else: | |||
| if isinstance(value, DataFrame): | |||
| if isinstance(value, DataFrame) and self.columns.is_unique: | |||
There was a problem hiding this comment.
will there be a problem on L3230 if value.columns is not unique?
There was a problem hiding this comment.
Depends of the key references a dup column
There was a problem hiding this comment.
let me rephrase this:
We don't get there, becasue 3227 would raise. If we would change the condition to recognize duplicates, we would get a problem in 3229, because the lenght of both lists would not match.
If we also fix this, we have to check that in case of a key which is duplicated in the columns, we select two columns and not only one in 3230 on the rhs
Edit: Similar problem if rhs would have duplicates.
There was a problem hiding this comment.
yah the other case im now unsure of is not all(x in self.columns for x in key)
There was a problem hiding this comment.
Yikes, this has ugly side effects. We have to find a way to handle duplicates there instead of dispatching down :(
And improve the test coverage of these cases
Have to check how many columns key references before raising (duplicates or MultiIndex may reference more than one column)