BUG: fix combine_first converting timestamp to int

[nixphix]

• closes combine_first returns unexpected results for timestamp dataframes #28481
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

This fix introduced two regression, but it appears like the fix only made the API consistent. Previously the failing regressions where inconsistent say df1.combine_first(df2) would not return the same result as df2.combine_first(df1) for the failing cases, more on these in the code comments.

Let me know if there is a better way to handle this.

[jreback]

can you post an example of what the change to a user would look like (e.g. what the 'regression' )is

[nixphix]

can you post an example of what the change to a user would look like (e.g. what the 'regression' )is

@jreback master is inconsistent when the other and self df's are swapped this is not captured by current test cases

# master
import pandas as pd
import numpy as np

df1 = pd.DataFrame([[np.nan, 3.0, True], [-4.6, np.nan, True], [np.nan, 7.0, False]])
df2 = pd.DataFrame([[-42.6, np.nan, True], [-5.0, 1.6, False]], index=[1, 2])

expected = pd.Series([True, True, False], name=2)
result1 = df1.combine_first(df2)[2]
result2 = df2.combine_first(df1)[2]

print(expected.dtype, result1.dtype, result2.dtype)
>>>bool bool object

# master
import pandas as pd

df1 = pd.DataFrame({"a": [0, 1, 3, 5]}, dtype="int64")
df2 = pd.DataFrame({"a": [1, 4]}, dtype="int64")

res1 = df1.combine_first(df2)
res2 = df2.combine_first(df1)

print(res1["a"].dtype, res2["a"].dtype)
>>>int64 float64

the current fix makes the function consistent but does make the result different from expected, in the first case now both ways we get a series of type object and the second case returns float64 either way.

# fix
import pandas as pd
import numpy as np

df1 = pd.DataFrame([[np.nan, 3.0, True], [-4.6, np.nan, True], [np.nan, 7.0, False]])
df2 = pd.DataFrame([[-42.6, np.nan, True], [-5.0, 1.6, False]], index=[1, 2])

expected = pd.Series([True, True, False], name=2)
result1 = df1.combine_first(df2)[2]
result2 = df2.combine_first(df1)[2]

print(expected.dtype, result1.dtype, result2.dtype)
>>>bool object object

# fix
import pandas as pd

df1 = pd.DataFrame({"a": [0, 1, 3, 5]}, dtype="int64")
df2 = pd.DataFrame({"a": [1, 4]}, dtype="int64")

res1 = df1.combine_first(df2)
res2 = df2.combine_first(df1)

print(res1["a"].dtype, res2["a"].dtype)
>>>float64 float64

let me know your thought on this.

[jbrockmendel]

@nixphix this looks pretty good. can you merge master and we'll see if the CI passes

[nixphix]

@jreback I have made the code changes but the ci is failing on an unrelated test case test_2d_to_1d_assignment_raises. Believe this is caused by breaking changes in dependency or the issue is in the master.

[github-actions]

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

[jreback]

looks good. if you can parameterize those tests and ping on green.

[jreback]

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

[jreback]

can you parameterize this as well

[jreback]

@nixphix lgtm, can you merge master and address the test comments, ping on green.

[jreback]

and make a seaparte test

[jreback]

this PR is prob ok, just needs a rebase and updating for comments.

[jbrockmendel]

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

[arw2019]

Closing in favor of #38145