-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
REGR: append tz-aware DataFrame with tz-naive values #36115
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
Changes from 5 commits
6acc0e2
15d024e
95d944b
7625a10
36b62cb
0534791
0d4a21c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -1110,6 +1110,23 @@ def test_append_empty_frame_to_series_with_dateutil_tz(self): | |||||||
result = df.append([s, s], ignore_index=True) | ||||||||
tm.assert_frame_equal(result, expected) | ||||||||
|
||||||||
def test_append_empty_tz_frame_with_datetime64ns(self): | ||||||||
# https://github.com/pandas-dev/pandas/issues/35460 | ||||||||
df = pd.DataFrame(columns=["a"]).astype("datetime64[ns, UTC]") | ||||||||
|
||||||||
# pd.NaT gets inferred as tz-naive, so append result is tz-naive | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it obvious this is the correct behavior? id expect object dtype result There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, good point. Yes, it should indeed give object dtype (at least that's how we concat tz-naive/tz-aware otherwise). Will need to check why it becomes datetime64 here .. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, so the reason is that we drop the empty arrays in pandas/pandas/core/dtypes/concat.py Lines 139 to 141 in 1a21836
so the empty tz-aware part is dropped, and the concat further only sees the tz-naive NaT. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jbrockmendel thoughts on this? This PR is for sure fixing an AssertionError that otherwise bubbled to the user (and which was a regression), which should not happen, but not fully sure what we should do with the expected result. |
||||||||
result = df.append({"a": pd.NaT}, ignore_index=True) | ||||||||
expected = pd.DataFrame({"a": [pd.NaT]}).astype("datetime64[ns]") | ||||||||
tm.assert_frame_equal(result, expected) | ||||||||
|
||||||||
# also test with typed value to append | ||||||||
df = pd.DataFrame(columns=["a"]).astype("datetime64[ns, UTC]") | ||||||||
result = df.append( | ||||||||
pd.Series({"a": pd.NaT}, dtype="datetime64[ns]"), ignore_index=True | ||||||||
) | ||||||||
expected = pd.DataFrame({"a": [pd.NaT]}).astype("datetime64[ns]") | ||||||||
tm.assert_frame_equal(result, expected) | ||||||||
|
||||||||
|
||||||||
class TestConcatenate: | ||||||||
def test_concat_copy(self): | ||||||||
|
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.
would remove the assert as its commented (or actually uncomment ok either way)