-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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: pd.concat
dataframes with different datetime64 resolutions
#53641
Conversation
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
I can confirm this issue still exists in pandas 2.1.1. This bug together with the latest changes in pyarrow 13.0.0 apache/arrow#38171 makes it difficult to use t1 = '2023-09-01'
t2 = '2023-09-01 01:00:00'
t3 = '2023-09-01 01:30:00'
ds1 = pd.date_range(t1, t1, freq='30T')
ds2 = pd.date_range(t2, t3, freq='30T')
df1 = pd.DataFrame({
'ds': ds1.astype('datetime64[us]'),
'y': list(range(len(ds1))),
})
df2 = pd.DataFrame({
'ds': ds2.astype('datetime64[ns]'),
'y': list(range(len(ds2))),
})
pd.concat([df1, df2], axis=0) And the error
|
@@ -858,3 +858,17 @@ def test_concat_multiindex_with_category(): | |||
) | |||
expected = expected.set_index(["c1", "c2"]) | |||
tm.assert_frame_equal(result, expected) | |||
|
|||
|
|||
def test_concat_datetime64_diff_resolution(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be simpler or more thorough to take existing tests in this file and parametrize them over units?
@jbrockmendel Yes, I can confirm that #55067 and #55374 raise no ValueError with this PR. Also, I tried what you suggested, i.e., parametrize existing tests but find it too hard to realize. Currently I made a slightly more comprehensive test than the previous version and parametrized it with different datetime resolutions. Can you check if that suffices? |
I suggest defining these fixtures in tests.reshape.concat.test_datetimes
Then pass these fixtures to test_concat_tz_series3 and cast Updating other tests in this file to use the same pattern would be optional. |
A few comments, otherwise looking good |
Done, thanks for the review @jbrockmendel! |
Thanks @Charlie-XIAO |
@mroeschke Also, would you be open to backporting this to 2.1.x? |
Ah, yeah +1 to backport this to 2.1.x |
@meeseeksdev backport 2.1.x. |
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
…ent datetime64 resolutions
@lithomas1 - This issue still exists on 2.1.4, was the backport included in the release? |
Unfortunately not, there seem to be other changes necessary in addition to this one to patch the bug for the 2.1 series. If you're curious, my attempt to do it was here #56311. |
pd.concat
dataframes with different datetime64 types #53640