BUG: Allow custom error values in parse_dates argument of read_sql like functions (GH35185) #37823
Conversation
avinashpancham
changed the title
| ], | ||
| ) | ||
| def test_custom_dateparsing_error(self, read_sql, text, mode): | ||
| if self.mode in mode: |
There was a problem hiding this comment.
can you make this tests assert the results, otherwise its not actually testing anything
pandas/io/sql.py
Outdated
| @@ -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.
can you document what values format can actualy take on here? also don't type ignore.
There was a problem hiding this comment.
WDYM with "values format can take on here", should I write all the arguments that to_datetime accepts?
I added the type ignore since to_datetime is not overloaded for this argument combination and therefore results into a mypy error. I did not want to add an overload variation for this exception, so any tips on how to approach this?
There was a problem hiding this comment.
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.
|
@jreback could you have another look? |
doc/source/whatsnew/v1.2.0.rst
Outdated
| @@ -745,6 +745,7 @@ I/O | |||
| - :meth:`read_fwf` was inferring compression with ``compression=None`` which was not consistent with the other :meth:``read_*`` functions (:issue:`37909`) | |||
| - :meth:`DataFrame.to_html` was ignoring ``formatters`` argument for ``ExtensionDtype`` columns (:issue:`36525`) | |||
| - Bumped minimum xarray version to 0.12.3 to avoid reference to the removed ``Panel`` class (:issue:`27101`) | |||
| - Allow custom error values for parse_dates argument of :func:`read_sql`, :func:`read_sql_query` and :func:`read_sql_table` (:issue:`GH35185`) | |||
There was a problem hiding this comment.
Could you move this to whatsnew/v1.3.0.rst?
pandas/tests/io/test_sql.py
Outdated
| "DateCol": {"errors": error}, | ||
| }, | ||
| ) | ||
| assert issubclass(df.DateCol.dtype.type, np.datetime64) |
There was a problem hiding this comment.
Could you test the full expected dataframe with tm.assert_frame_equal?
There was a problem hiding this comment.
Np to do this, however all the other tests that use 'types_test_data' do not use tm.assert_frame_equal, but the assert approach I used. So just doublechecking if we want to go ahead with this.
There was a problem hiding this comment.
assert_frame_equal is preferable in general since it tests the asserts you have and more
There was a problem hiding this comment.
Updated the test, but sometimes the assert fails since the machine casts the integer that is read to int32. Giving the error:
E Attribute "dtype" are different
E [left]: int64
E [right]: int32
I've been trying some stuff, but without success. Is there a way to let pandas decide which int type to use, so that we don't have this mismatch? Else we can provide the argument check_dtype=False to assert_frame_equal, but IMO that seems a bit too much.
There was a problem hiding this comment.
nvm, it looks like I got it
|
@avinashpancham ok this is good. Can you do a followon PR (or open an issue to do so), that adds examples to the |
|
Sure, will create a follow on PR to add some examples in the docstring. |
|
Thanks @avinashpancham |
…ke functions (GH35185) (pandas-dev#37823) * BUG: Allow custom error values in parse_dates argument of read_sql (GH35185) * Ignore call overload for one time exception * Add tests for custom dateparsing error for read_sql * Generalize test for all sql read functions * Add conditional mode for tests * Typo * Updated test * Update to_datetime call in _handle_date_column * Move whatsnew message to v1.3.0 * Update test * Explicit cast to int64 * Remove accidental check_dtype=False * Fix wrong reference in whatsnew * Add hyphen Co-authored-by: Jeff Reback <[email protected]>
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff