Skip to content
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.

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

BUG: fix combine_first converting timestamp to int #35514

Closed
wants to merge 11 commits into from
1 change: 1 addition & 0 deletions doc/source/whatsnew/v1.2.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ Categorical

Datetimelike
^^^^^^^^^^^^
- Bug in :meth:`DataFrame.combine_first` that would convert datetime-like column on other :class:`DataFrame` to integer when the column is not present in original :class:`DataFrame` (:issue:`28481`)
- Bug in :attr:`DatetimeArray.date` where a ``ValueError`` would be raised with a read-only backing array (:issue:`33530`)
- Bug in ``NaT`` comparisons failing to raise ``TypeError`` on invalid inequality comparisons (:issue:`35046`)
- Bug in :class:`DateOffset` where attributes reconstructed from pickle files differ from original objects when input values exceed normal ranges (e.g months=12) (:issue:`34511`)
Expand Down
2 changes: 1 addition & 1 deletion pandas/core/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -6169,7 +6169,7 @@ def combine(
otherSeries = otherSeries.astype(new_dtype)

arr = func(series, otherSeries)
arr = maybe_downcast_to_dtype(arr, this_dtype)
arr = maybe_downcast_to_dtype(arr, new_dtype)

result[col] = arr

Expand Down
35 changes: 30 additions & 5 deletions pandas/tests/frame/methods/test_combine_first.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,14 @@ def test_combine_first_mixed_bug(self):
)
df2 = DataFrame([[-42.6, np.nan, True], [-5.0, 1.6, False]], index=[1, 2])

result = df1.combine_first(df2)[2]
expected = Series([True, True, False], name=2)
tm.assert_series_equal(result, expected)
expected1 = pd.Series([True, True, False], name=2, dtype=object)
nixphix marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

can you parameterize this test (e.g. put the df1 and df2 in the parameter along with the expected)

expected2 = pd.Series([True, True, False], name=2, dtype=object)

result1 = df1.combine_first(df2)[2]
result2 = df2.combine_first(df1)[2]
nixphix marked this conversation as resolved.
Show resolved Hide resolved

tm.assert_series_equal(result1, expected1)
tm.assert_series_equal(result2, expected2)

# GH 3593, converting datetime64[ns] incorrectly
df0 = DataFrame(
Expand Down Expand Up @@ -340,8 +345,9 @@ def test_combine_first_int(self):
df2 = pd.DataFrame({"a": [1, 4]}, dtype="int64")

res = df1.combine_first(df2)
tm.assert_frame_equal(res, df1)
assert res["a"].dtype == "int64"
res2 = df1.combine_first(df2)

assert res["a"].dtype == res2["a"].dtype
nixphix marked this conversation as resolved.
Show resolved Hide resolved

@pytest.mark.parametrize("val", [1, 1.0])
def test_combine_first_with_asymmetric_other(self, val):
Expand All @@ -353,3 +359,22 @@ def test_combine_first_with_asymmetric_other(self, val):
exp = pd.DataFrame({"isBool": [True], "isNum": [val]})

tm.assert_frame_equal(res, exp)


@pytest.mark.parametrize(
"val1, val2",
[
(datetime(2020, 1, 1), datetime(2020, 1, 2)),
(pd.Period("2020-01-01", "D"), pd.Period("2020-01-02", "D")),
(pd.Timedelta("89 days"), pd.Timedelta("60 min")),
],
)
def test_combine_first_timestamp_bug(val1, val2, nulls_fixture):
nixphix marked this conversation as resolved.
Show resolved Hide resolved

Copy link
Member

Choose a reason for hiding this comment

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

can you add a comment here "# GH#35514"

df1 = pd.DataFrame([[nulls_fixture, nulls_fixture]], columns=["a", "b"])
df2 = pd.DataFrame([[val1, val2]], columns=["b", "c"])

res = df1.combine_first(df2)
exp = pd.DataFrame([[nulls_fixture, val1, val2]], columns=["a", "b", "c"])

tm.assert_frame_equal(res, exp)