-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Allow custom error values in parse_dates argument of read_sql like functions (GH35185) #37823
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 8 commits
33bdac6
bfdc016
ff90c17
e451c0e
a7637d7
7d8a5b0
c2b4b2e
1d3d25a
c4597b6
2054360
7673ed8
03d05d7
ada00a1
42fd388
99d4bb1
0077f6b
ddcf646
a2b6635
37af90a
664e97a
29a7c26
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 |
---|---|---|
|
@@ -79,7 +79,9 @@ def _process_parse_dates_argument(parse_dates): | |
|
||
def _handle_date_column(col, utc=None, format=None): | ||
if isinstance(format, dict): | ||
return to_datetime(col, errors="ignore", **format) | ||
return to_datetime( | ||
col, **{"errors": "ignore", **format} # type: ignore[call-overload] | ||
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. can you document what values format can actualy take on here? also don't type ignore. 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. WDYM with "values format can take on here", should I write all the arguments that I added the type ignore since 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. Updated the PR, is this what you meant with documenting values that format can take? Also changed the python code so that we dont have to type ignore anymore. |
||
) | ||
else: | ||
# Allow passing of formatting string for integers | ||
# GH17855 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -741,6 +741,30 @@ def test_date_parsing(self): | |
Timestamp("2010-12-12"), | ||
] | ||
|
||
@pytest.mark.parametrize( | ||
"read_sql, text, mode", | ||
[ | ||
(sql.read_sql, "SELECT * FROM types_test_data", ("sqlalchemy", "fallback")), | ||
(sql.read_sql, "types_test_data", ("sqlalchemy")), | ||
( | ||
sql.read_sql_query, | ||
"SELECT * FROM types_test_data", | ||
("sqlalchemy", "fallback"), | ||
), | ||
(sql.read_sql_table, "types_test_data", ("sqlalchemy")), | ||
], | ||
) | ||
def test_custom_dateparsing_error(self, read_sql, text, mode): | ||
if self.mode in mode: | ||
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. can you make this tests assert the results, otherwise its not actually testing anything 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. Done |
||
read_sql( | ||
text, | ||
con=self.conn, | ||
parse_dates={ | ||
"DateCol": {"errors": "coerce"}, | ||
"IntDateCol": {"errors": "ignore"}, | ||
}, | ||
) | ||
|
||
def test_date_and_index(self): | ||
# Test case where same column appears in parse_date and index_col | ||
|
||
|
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.
Could you move this to
whatsnew/v1.3.0.rst
?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.
Done