-
-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
ENH: Support writing timestamps with timezones with to_sql #22654
Changes from 8 commits
776240b
befd200
e9f122f
969d2da
cc79b90
24dbaa5
6e86d58
c7c4a7a
6aa4878
513bbc8
58772e1
1a29148
96e9188
ded5584
d575089
a7d1b3e
305759c
7a79531
24823f8
7db4eaa
76e46dc
978a0d3
8025248
0e89370
bab5cfb
de62788
e940279
8c754b5
e85842f
5af83f7
6b3a3f1
c4304ec
1054fdb
f21c755
f872ff7
ef3b20f
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 |
---|---|---|
|
@@ -954,7 +954,8 @@ def test_sqlalchemy_type_mapping(self): | |
utc=True)}) | ||
db = sql.SQLDatabase(self.conn) | ||
table = sql.SQLTable("test_type", db, frame=df) | ||
assert isinstance(table.table.c['time'].type, sqltypes.DateTime) | ||
# GH 9086: TIMESTAMP is the suggested type for datetimes with timezones | ||
assert isinstance(table.table.c['time'].type, sqltypes.TIMESTAMP) | ||
|
||
def test_database_uri_string(self): | ||
|
||
|
@@ -1354,9 +1355,32 @@ def check(col): | |
df = sql.read_sql_table("types_test_data", self.conn) | ||
check(df.DateColWithTz) | ||
|
||
def test_datetime_with_timezone_writing(self): | ||
# GH 9086 | ||
if self.flavor != 'postgresql': | ||
msg = "{} does not support datetime with time zone" | ||
pytest.skip(msg.format(self.flavor)) | ||
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. Shouldn't we test assert the behaviour for a database that does not support it? Eg what happens if you write a column with timezone aware data to mysql? 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. so the values send change:
but not sure if that changes how the values are then stored in the mysql database (since the utc / naive time is still the same), but so this might be worth testing. 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. That's a good point. From earlier tests databases that don't support |
||
|
||
df = DataFrame({'A': date_range( | ||
'2013-01-01 09:00:00', periods=3, tz='US/Pacific' | ||
)}) | ||
df.to_sql('test_datetime_tz', self.conn) | ||
|
||
expected = df.copy() | ||
expected['A'] = expected['A'].dt.tz_convert('UTC') | ||
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. So that I remember correctly: when reading, and we get datetime objects with an offset (if there is a 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. That's correct, so we should expect this round trip test to load |
||
|
||
result = sql.read_sql_table('test_datetime_tz', self.conn) | ||
result = result.drop('index', axis=1) | ||
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. you can put |
||
tm.assert_frame_equal(result, expected) | ||
|
||
result = sql.read_sql_query( | ||
'SELECT * FROM test_datetime_tz', self.conn | ||
) | ||
result = result.drop('index', axis=1) | ||
tm.assert_frame_equal(result, expected) | ||
|
||
def test_date_parsing(self): | ||
# No Parsing | ||
df = sql.read_sql_table("types_test_data", self.conn) | ||
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. Shouldn't we rather test that it is not parsed instead of removing it? (but I agree this currently looks like this is not doing much) 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. Oh I see. My cursory glance thought it was duplicate of the line below; you're right, I will try to add an assert to this result as well |
||
|
||
df = sql.read_sql_table("types_test_data", self.conn, | ||
parse_dates=['DateCol']) | ||
|
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.
Should be