Repository: pandas-dev/pandas
Pull request: ENH: try to preserve the dtype on combine_first for the case where the two DataFrame objects have the same columns
…e two DataFrame objects have the same columns

- [ ] closes #7509 
- [ ] tests added / passed
- [ ] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [ ] whatsnew entry


Problem statement:
combine_first not retaining dtypes
I found a number of issues that seemed related, all closed over a year ago, but there still seem to be some inconsistencies here:

``` python
In [1]: from datetime import datetime
In [2]: import pandas as pd
In [3]: pd.__version__
Out[3]: '0.13.1'
In [4]: dfa = pd.DataFrame([[datetime.now(), 2]], columns=['a','b'])
In [5]: dfb = pd.DataFrame([[4],[5]], columns=['b'])
In [6]: dfa.dtypes
Out[6]: 
a    datetime64[ns]
b             int64
dtype: object
In [7]: dfb.dtypes
Out[7]: 
b    int64
dtype: object
In [8]: # int64 becomes float64 when combining the two frames
In [9]: dfa.combine_first(dfb).dtypes
Out[9]: 
a    datetime64[ns]
b           float64
dtype: object
In [10]: # datetime64[ns] becomes float64 if the first frame is empty
In [11]: dfa.iloc[:0].combine_first(dfb).dtypes
Out[11]: 
a    float64
b      int64
dtype: object
```

Hints from issue discussion:
can you ref the issues?

My search turned up issues #3041, #3043, #3552, and #3555

(whoa, github's auto-complete on those issue numbers seems totally unrelated...)

none of those address empty types (and thus their are prob no tests).

care to do a pull-request to put in tests/fix?

I haven't looked under the hood; I've no clue how to go about fixing this.  Also, there seem to be two separate problems here (perhaps I should have opened two issues):
1. Specifically when the two DataFrames are both NOT empty, it changes int64 to float64.
2. When the one with the datetime column IS empty, the datetime dtype is not preserved.

the first is not feasible to fix, since `int` CANNOT hold `nan`. It is very tricky to convert this to float, then convert back if necessary (and hits perf).

The 2nd could be fixed (as `datetime64[ns]` CAN hold na (via `NaT`))

why don't you write up some tests then....get's you started :)

Oh, of course.  The `float64` is needed for the `nan`.  But if `nan` does not result, the `int64` is retained:

``` python
In [3]: dfa = pd.DataFrame([[datetime.now(), 2]], columns=['a','b'])
In [4]: dfb = pd.DataFrame([[4]], columns=['b'])
In [5]: dfa.combine_first(dfb).dtypes
Out[5]: 
a    datetime64[ns]
b             int64
dtype: object
```

I should have thought of that.

When you combine_first the original float32 column is transformed to float64:

``` python
"""Example of Pandas bug."""
from pandas import DataFrame
from numpy import float32

print('-' * 15)
d1 = DataFrame(index=[0])
d1['A'] = [3.5]
d1['A'] = d1['A'].astype(float32)
print(d1.dtypes)
print('-' * 15)
d2 = DataFrame(index=[0])
d2['B'] = [35]  # if uncomment this line the result is correct, nonsense for me
d2 = d2.combine_first(d1)
print(d2.dtypes)
```

Current output:

```
---------------
A    float32
dtype: object
---------------
A    float64
B      int64
dtype: object
```

Expected output:

```
---------------
A    float32
dtype: object
---------------
A    float32
B      int64
dtype: object
```

This is still a bug in 2020:

```python
>>> dfa = pd.DataFrame({"A": [0, 1, 2]}, index=[0, 1, 2])
>>> dfb = pd.DataFrame({"A": [7, 8, 9]}, index=[1, 2, 3])
>>> dfa.combine_first(dfb).dtypes  # Expect to see int64
A    float64
dtype: object
```

Any plans on addressing this issue?
pandas is an all volunteer project.

if you would like to submit a PR then one of the volunteers would be able to code review 
> pandas is an all volunteer project.
> 
> if you would like to submit a PR then one of the volunteers would be able to code review

I think think this is a serious issue. If I had the knowledge to fix it myself I would. Maybe someone from the core developers can have a look

Commit message: remove redundant check

Patch to review:
diff --git a/doc/source/whatsnew/v1.3.0.rst b/doc/source/whatsnew/v1.3.0.rst
index cb97fdeccd579..eba01f609ef0c 100644
--- a/doc/source/whatsnew/v1.3.0.rst
+++ b/doc/source/whatsnew/v1.3.0.rst
@@ -65,6 +65,53 @@ Notable bug fixes
 These are bug fixes that might have notable behavior changes.
 
 
+Preserve dtypes in  :meth:`~pandas.DataFrame.combine_first`
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+:meth:`~pandas.DataFrame.combine_first` will now preserve dtypes (:issue:`7509`)
+
+.. ipython:: python
+
+   df1 = pd.DataFrame({"A": [1, 2, 3], "B": [1, 2, 3]}, index=[0, 1, 2])
+   df2 = pd.DataFrame({"B": [4, 5, 6], "C": [1, 2, 3]}, index=[2, 3, 4])
+   combined = df1.combine_first(df2)
+
+*pandas 1.2.x*
+
+.. code-block:: ipython
+
+   In [1]: (combined, "---------------", combined.dtypes)
+   Out[2]:
+   (     A    B    C
+   0  1.0  1.0  NaN
+   1  2.0  2.0  NaN
+   2  3.0  3.0  1.0
+   3  NaN  5.0  2.0
+   4  NaN  6.0  3.0,
+   '---------------',
+   A    float64
+   B    float64
+   C    float64
+   dtype: object)
+
+*pandas 1.3.0*
+
+.. ipython:: python
+
+   In [1]: (combined, "---------------", combined.dtypes)
+   Out[2]:
+   (     A  B    C
+   0  1.0  1  NaN
+   1  2.0  2  NaN
+   2  3.0  3  1.0
+   3  NaN  5  2.0
+   4  NaN  6  3.0, 
+   '---------------', 
+   A    float64
+   B      int64
+   C    float64
+   dtype: object)
+
 
 .. _whatsnew_130.api_breaking.deps:
 
diff --git a/pandas/core/frame.py b/pandas/core/frame.py
index e65e9302dd4d5..897ff7e12cc9e 100644
--- a/pandas/core/frame.py
+++ b/pandas/core/frame.py
@@ -6482,7 +6482,18 @@ def combiner(x, y):
 
             return expressions.where(mask, y_values, x_values)
 
-        return self.combine(other, combiner, overwrite=False)
+        combined = self.combine(other, combiner, overwrite=False)
+
+        dtypes = {
+            col: find_common_type([self.dtypes[col], other.dtypes[col]])
+            for col in self.columns.intersection(other.columns)
+            if not is_dtype_equal(combined.dtypes[col], self.dtypes[col])
+        }
+
+        if dtypes:
+            combined = combined.astype(dtypes)
+
+        return combined
 
     def update(
         self,
diff --git a/pandas/tests/frame/methods/test_combine_first.py b/pandas/tests/frame/methods/test_combine_first.py
index 934ad9eb8213a..e3a1d5792d24d 100644
--- a/pandas/tests/frame/methods/test_combine_first.py
+++ b/pandas/tests/frame/methods/test_combine_first.py
@@ -3,6 +3,8 @@
 import numpy as np
 import pytest
 
+from pandas.core.dtypes.cast import find_common_type, is_dtype_equal
+
 import pandas as pd
 from pandas import DataFrame, Index, MultiIndex, Series
 import pandas._testing as tm
@@ -18,9 +20,7 @@ def test_combine_first_mixed(self):
         b = Series(range(2), index=range(5, 7))
         g = DataFrame({"A": a, "B": b})
 
-        exp = DataFrame(
-            {"A": list("abab"), "B": [0.0, 1.0, 0.0, 1.0]}, index=[0, 1, 5, 6]
-        )
+        exp = DataFrame({"A": list("abab"), "B": [0, 1, 0, 1]}, index=[0, 1, 5, 6])
         combined = f.combine_first(g)
         tm.assert_frame_equal(combined, exp)
 
@@ -144,7 +144,7 @@ def test_combine_first_return_obj_type_with_bools(self):
         )
         df2 = DataFrame([[-42.6, np.nan, True], [-5.0, 1.6, False]], index=[1, 2])
 
-        expected = Series([True, True, False], name=2, dtype=object)
+        expected = Series([True, True, False], name=2, dtype=bool)
 
         result_12 = df1.combine_first(df2)[2]
         tm.assert_series_equal(result_12, expected)
@@ -157,22 +157,22 @@ def test_combine_first_return_obj_type_with_bools(self):
         (
             (
                 [datetime(2000, 1, 1), datetime(2000, 1, 2), datetime(2000, 1, 3)],
-                [None, None, None],
+                [pd.NaT, pd.NaT, pd.NaT],
                 [datetime(2000, 1, 1), datetime(2000, 1, 2), datetime(2000, 1, 3)],
             ),
             (
-                [None, None, None],
+                [pd.NaT, pd.NaT, pd.NaT],
                 [datetime(2000, 1, 1), datetime(2000, 1, 2), datetime(2000, 1, 3)],
                 [datetime(2000, 1, 1), datetime(2000, 1, 2), datetime(2000, 1, 3)],
             ),
             (
-                [datetime(2000, 1, 2), None, None],
+                [datetime(2000, 1, 2), pd.NaT, pd.NaT],
                 [datetime(2000, 1, 1), datetime(2000, 1, 2), datetime(2000, 1, 3)],
                 [datetime(2000, 1, 2), datetime(2000, 1, 2), datetime(2000, 1, 3)],
             ),
             (
                 [datetime(2000, 1, 1), datetime(2000, 1, 2), datetime(2000, 1, 3)],
-                [datetime(2000, 1, 2), None, None],
+                [datetime(2000, 1, 2), pd.NaT, pd.NaT],
                 [datetime(2000, 1, 1), datetime(2000, 1, 2), datetime(2000, 1, 3)],
             ),
         ),
@@ -196,13 +196,13 @@ def test_combine_first_align_nan(self):
 
         res = dfa.combine_first(dfb)
         exp = DataFrame(
-            {"a": [pd.Timestamp("2011-01-01"), pd.NaT], "b": [2.0, 5.0]},
+            {"a": [pd.Timestamp("2011-01-01"), pd.NaT], "b": [2, 5]},
             columns=["a", "b"],
         )
         tm.assert_frame_equal(res, exp)
         assert res["a"].dtype == "datetime64[ns]"
         # ToDo: this must be int64
-        assert res["b"].dtype == "float64"
+        assert res["b"].dtype == "int64"
 
         res = dfa.iloc[:0].combine_first(dfb)
         exp = DataFrame({"a": [np.nan, np.nan], "b": [4, 5]}, columns=["a", "b"])
@@ -219,14 +219,12 @@ def test_combine_first_timezone(self):
             columns=["UTCdatetime", "abc"],
             data=data1,
             index=pd.date_range("20140627", periods=1),
-            dtype="object",
         )
         data2 = pd.to_datetime("20121212 12:12").tz_localize("UTC")
         df2 = DataFrame(
             columns=["UTCdatetime", "xyz"],
             data=data2,
             index=pd.date_range("20140628", periods=1),
-            dtype="object",
         )
         res = df2[["UTCdatetime"]].combine_first(df1)
         exp = DataFrame(
@@ -239,13 +237,10 @@ def test_combine_first_timezone(self):
             },
             columns=["UTCdatetime", "abc"],
             index=pd.date_range("20140627", periods=2, freq="D"),
-            dtype="object",
         )
         assert res["UTCdatetime"].dtype == "datetime64[ns, UTC]"
         assert res["abc"].dtype == "datetime64[ns, UTC]"
-        # Need to cast all to "obejct" because combine_first does not retain dtypes:
-        # GH Issue 7509
-        res = res.astype("object")
+
         tm.assert_frame_equal(res, exp)
 
         # see gh-10567
@@ -360,12 +355,11 @@ def test_combine_first_int(self):
         df2 = DataFrame({"a": [1, 4]}, dtype="int64")
 
         result_12 = df1.combine_first(df2)
-        expected_12 = DataFrame({"a": [0, 1, 3, 5]}, dtype="float64")
+        expected_12 = DataFrame({"a": [0, 1, 3, 5]})
         tm.assert_frame_equal(result_12, expected_12)
 
         result_21 = df2.combine_first(df1)
-        expected_21 = DataFrame({"a": [1, 4, 3, 5]}, dtype="float64")
-
+        expected_21 = DataFrame({"a": [1, 4, 3, 5]})
         tm.assert_frame_equal(result_21, expected_21)
 
     @pytest.mark.parametrize("val", [1, 1.0])
@@ -404,11 +398,38 @@ def test_combine_first_string_dtype_only_na(self):
 def test_combine_first_timestamp_bug(scalar1, scalar2, nulls_fixture):
     # GH28481
     na_value = nulls_fixture
+
     frame = DataFrame([[na_value, na_value]], columns=["a", "b"])
     other = DataFrame([[scalar1, scalar2]], columns=["b", "c"])
 
+    common_dtype = find_common_type([frame.dtypes["b"], other.dtypes["b"]])
+
+    if is_dtype_equal(common_dtype, "object") or frame.dtypes["b"] == other.dtypes["b"]:
+        val = scalar1
+    else:
+        val = na_value
+
     result = frame.combine_first(other)
-    expected = DataFrame([[na_value, scalar1, scalar2]], columns=["a", "b", "c"])
+
+    expected = DataFrame([[na_value, val, scalar2]], columns=["a", "b", "c"])
+
+    expected["b"] = expected["b"].astype(common_dtype)
+
+    tm.assert_frame_equal(result, expected)
+
+
+def test_combine_first_timestamp_bug_NaT():
+    # GH28481
+    frame = DataFrame([[pd.NaT, pd.NaT]], columns=["a", "b"])
+    other = DataFrame(
+        [[datetime(2020, 1, 1), datetime(2020, 1, 2)]], columns=["b", "c"]
+    )
+
+    result = frame.combine_first(other)
+    expected = DataFrame(
+        [[pd.NaT, datetime(2020, 1, 1), datetime(2020, 1, 2)]], columns=["a", "b", "c"]
+    )
+
     tm.assert_frame_equal(result, expected)
 
 
@@ -439,3 +460,25 @@ def test_combine_first_with_nan_multiindex():
         index=mi_expected,
     )
     tm.assert_frame_equal(res, expected)
+
+
+def test_combine_preserve_dtypes():
+    # GH7509
+    a = Series(["a", "b"], index=range(2))
+    b = Series(range(2), index=range(2))
+    f = DataFrame({"A": a, "B": b})
+
+    c = Series(["a", "b"], index=range(5, 7))
+    b = Series(range(-1, 1), index=range(5, 7))
+    g = DataFrame({"B": b, "C": c})
+
+    exp = DataFrame(
+        {
+            "A": ["a", "b", np.nan, np.nan],
+            "B": [0, 1, -1, 0],
+            "C": [np.nan, np.nan, "a", "b"],
+        },
+        index=[0, 1, 5, 6],
+    )
+    combined = f.combine_first(g)
+    tm.assert_frame_equal(combined, exp)


Code review comments:
diff_hunk: @@ -65,6 +65,53 @@ Notable bug fixes
 These are bug fixes that might have notable behavior changes.
 
 
+Preserve dtypes in  :meth:`~pandas.DataFrame.combine_first`
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+:meth:`~pandas.DataFrame.combine_first` will now preserve dtypes (:issue:`7509`)
+
+.. ipython:: python
+
+   df1 = pd.DataFrame({"A": [1, 2, 3], "B": [1, 2, 3]}, index=[0, 1, 2])
+   df2 = pd.DataFrame({"B": [4, 5, 6], "C": [1, 2, 3]}, index=[2, 3, 4])
+   combined = df1.combine_first(df2)
+
+*pandas 1.2.x*
+
+.. code-block:: ipython
+
+   In [1]: (combined, "---------------", combined.dtypes)
+   Out[2]:
+   (     A    B    C
+   0  1.0  1.0  NaN
+   1  2.0  2.0  NaN
+   2  3.0  3.0  1.0
+   3  NaN  5.0  2.0
+   4  NaN  6.0  3.0,
+   '---------------',
+   A    float64
+   B    float64
+   C    float64
+   dtype: object)
+
+*pandas 1.3.0*
+
+.. ipython:: python
+
+   In [1]: (combined, "---------------", combined.dtypes)
path: doc/source/whatsnew/v1.3.0.rst
text: @user1:
don't add ipython prompts, just the code

diff_hunk: @@ -65,6 +65,53 @@ Notable bug fixes
 These are bug fixes that might have notable behavior changes.
 
 
+Preserve dtypes in  :meth:`~pandas.DataFrame.combine_first`
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+:meth:`~pandas.DataFrame.combine_first` will now preserve dtypes (:issue:`7509`)
+
+.. ipython:: python
+
+   df1 = pd.DataFrame({"A": [1, 2, 3], "B": [1, 2, 3]}, index=[0, 1, 2])
+   df2 = pd.DataFrame({"B": [4, 5, 6], "C": [1, 2, 3]}, index=[2, 3, 4])
+   combined = df1.combine_first(df2)
+
+*pandas 1.2.x*
+
+.. code-block:: ipython
+
+   In [1]: (combined, "---------------", combined.dtypes)
path: doc/source/whatsnew/v1.3.0.rst
text: @user1:
don't need this extra printing, you can just whos `combine.dtypes`