Backport PR #47581 on branch 1.4.x (BUG: DataFrame.loc not aligning rhs df for single block case) #47984
Conversation
Move whatsnew
There was a problem hiding this comment.
we need to check whether #47361 was a regression first as the merge conflicts here look like they are down to those changes.
yeah simpler to leave out the release note here. do a move on master, then just sync 1.4.4 release notes directly on 1.4.x (although we have a another backport on hold)
pandas/core/frame.py
Outdated
| elif is_dict_like(value): | ||
| return _reindex_for_setitem(Series(value), self.index) |
There was a problem hiding this comment.
this includes the changes from #47361, so should backport that first (I think) so that we include test_setitem_aligning_dict_with_index in the backport also
There was a problem hiding this comment.
and move that release note too (and update the milestones etc)
There was a problem hiding this comment.
but first need to confirm if that's also a regression (I can do that tomorrow)
There was a problem hiding this comment.
This was not a regression unfortunately, did not work on 1.3.5 either
simonjayhawkins
added
Indexing
pandas/core/frame.py
Outdated
| # or through loc single_block_path | ||
| if isinstance(value, DataFrame): | ||
| return _reindex_for_setitem(value, self.index) | ||
| elif isinstance(value, Series): |
There was a problem hiding this comment.
This looks a bit weird now, but is closer to main than combining this two branches
There was a problem hiding this comment.
right so the comment # We should never get here with DataFrame value was not true?
the new comment refers to isetitem which is not on 1.4.x so slightly confusing.
it looks weird because could have just done if isinstance(value, (Series, DataFrame)): instead?
and the case of handling a dict-like value for the rhs was not working on 1.3.5 so we leaving that behavior unchanged here?
and slightly OT for the backport, on main we get a rogue warning?
FutureWarning: In a future version, `df.iloc[:, i] = newvals` will attempt to set the values inplace instead of always setting a new array. To retain the old behavior, use either `df[df.columns[i]] = newvals` or, if columns are non-unique, `df.isetitem(i, newvals)`
result.loc[:, "var"] = rhs
There was a problem hiding this comment.
right so the comment # We should never get here with DataFrame value was not true?
Yes, this was incorrect
the new comment refers to isetitem which is not on 1.4.x so slightly confusing.
good point forgot about this. will remove
it looks weird because could have just done if isinstance(value, (Series, DataFrame)): instead?
wanted to keep it closer to main, but can combine
and the case of handling a dict-like value for the rhs was not working on 1.3.5 so we leaving that behavior unchanged here?
correct
Yeah the warning is a bit annoying. Intend to look into this soonish
|
OK if not a regression then not necessarily an issue as we can still do bug fixes in patch releases. don't merge yet, I'll look again tomorrow. If we go with the original commit in this PR we should probably do it as 2 separate backports. |
|
Yep, if we do both then separate. I would just do the regression here. It's close to 1.5 anyway. The change compared to main is not that big |
Backport PR #47581
cc @simonjayhawkins
WIll put up a pr for the whatsnew later