Skip to content

BUG: fix combine_first reorders columns #60791

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 27, 2025
Merged

BUG: fix combine_first reorders columns #60791

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
44c9a14
Add test
yuanx749 Jan 26, 2025
e2f4afd
Fix combine_first reorders columns
yuanx749 Jan 26, 2025
aefa296
Add whatsnew
yuanx749 Jan 26, 2025
da59cdf
Merge remote-tracking branch 'upstream/main' into combine_first
yuanx749 Jan 26, 2025
58230c9
Fix corner case when self is empty and future.infer_string is True
yuanx749 Jan 26, 2025
cc61621
Update
yuanx749 Jan 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/source/whatsnew/v3.0.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,7 @@ Groupby/resample/rolling
Reshaping
^^^^^^^^^
- Bug in :func:`qcut` where values at the quantile boundaries could be incorrectly assigned (:issue:`59355`)
- Bug in :meth:`DataFrame.combine_first` not preserving the column order (:issue:`60427`)
- Bug in :meth:`DataFrame.join` inconsistently setting result index name (:issue:`55815`)
- Bug in :meth:`DataFrame.join` when a :class:`DataFrame` with a :class:`MultiIndex` would raise an ``AssertionError`` when :attr:`MultiIndex.names` contained ``None``. (:issue:`58721`)
- Bug in :meth:`DataFrame.merge` where merging on a column containing only ``NaN`` values resulted in an out-of-bounds array access (:issue:`59421`)
Expand Down
5 changes: 3 additions & 2 deletions pandas/core/frame.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8672,6 +8672,9 @@ def combine(
"""
other_idxlen = len(other.index) # save for compare

# preserve column order
new_columns = self.columns.union(other.columns, sort=False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to be done here as opposed to where it was before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it before this, other = self.align(other) because align can change the column order of other, as it sorts keys lexicographically.

But I think it is better to do it where it was before, after the early return. I've updated it in the new commit.


this, other = self.align(other)
new_index = this.index

Expand All @@ -8681,8 +8684,6 @@ def combine(
if self.empty and len(other) == other_idxlen:
return other.copy()

# sorts if possible; otherwise align above ensures that these are set-equal
new_columns = this.columns.union(other.columns)
do_fill = fill_value is not None
result = {}
for col in new_columns:
Expand Down
12 changes: 11 additions & 1 deletion pandas/tests/frame/methods/test_combine_first.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ def test_combine_first_with_asymmetric_other(self, val):
df2 = DataFrame({"isBool": [True]})

res = df1.combine_first(df2)
exp = DataFrame({"isBool": [True], "isNum": [val]})
exp = DataFrame({"isNum": [val], "isBool": [True]})

tm.assert_frame_equal(res, exp)

Expand Down Expand Up @@ -555,3 +555,13 @@ def test_combine_first_empty_columns():
result = left.combine_first(right)
expected = DataFrame(columns=["a", "b", "c"])
tm.assert_frame_equal(result, expected)


def test_combine_first_preserve_column_order():
# GH#60427
df1 = DataFrame({"B": [1, 2, 3], "A": [4, None, 6]})
df2 = DataFrame({"A": [5]}, index=[1])

result = df1.combine_first(df2)
expected = DataFrame({"B": [1, 2, 3], "A": [4.0, 5.0, 6.0]})
tm.assert_frame_equal(result, expected)
Loading