[BUG] Add is_coerce argument to func array_to_datetime_object (GH26122) #26257
Conversation
pandas/_libs/tslib.pyx
Outdated
| @@ -757,7 +758,8 @@ cdef inline ignore_errors_out_of_bounds_fallback(ndarray[object] values): | |||
|
|
|||
| @cython.wraparound(False) | |||
| @cython.boundscheck(False) | |||
| cdef array_to_datetime_object(ndarray[object] values, bint is_raise, | |||
| cdef array_to_datetime_object(ndarray[object] values, | |||
There was a problem hiding this comment.
really? we already pass thru errors so why do u need yet another arg?
There was a problem hiding this comment.
Agreed. Instead of passing is_raise& is_coerce just pass down errors from array_to_datetime. Please update the docstring of array_to_datetime_object after that change.
There was a problem hiding this comment.
What exactly is the objection to using is_raise and is_coerce?
We use them all over array_to_datetime. By passing through errors, you're essentially duplicating the logic array_to_datetime does to determine these flags, which you ultimately are going to use in array_to_datetime_object.
There was a problem hiding this comment.
That's fair. Since this is essentially a private method too changing the signature shouldn't be that big of a deal
There was a problem hiding this comment.
no, we already consistency pass around errors=, I think better here rather than having 2 args that are passed that could actually be in conflict.
Codecov Report
@@ Coverage Diff @@
## master #26257 +/- ##
==========================================
- Coverage 91.97% 91.97% -0.01%
==========================================
Files 175 175
Lines 52386 52386
==========================================
- Hits 48184 48180 -4
- Misses 4202 4206 +4
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #26257 +/- ##
==========================================
+ Coverage 91.97% 91.98% +<.01%
==========================================
Files 175 175
Lines 52386 52374 -12
==========================================
- Hits 48184 48175 -9
+ Misses 4202 4199 -3
Continue to review full report at Codecov.
|
doc/source/whatsnew/v0.25.0.rst
Outdated
| @@ -273,7 +273,7 @@ Datetimelike | |||
| - Bug in :class:`DataFrame` and :class:`Series` where timezone aware data with ``dtype='datetime64[ns]`` was not cast to naive (:issue:`25843`) | |||
| - Improved :class:`Timestamp` type checking in various datetime functions to prevent exceptions when using a subclassed ``datetime`` (:issue:`25851`) | |||
| - Bug in :class:`Series` and :class:`DataFrame` repr where ``np.datetime64('NaT')`` and ``np.timedelta64('NaT')`` with ``dtype=object`` would be represented as ``NaN`` (:issue:`25445`) | |||
| - | |||
| - Bug in :meth:`to_datetime` which does not replace the invalid argument with ``NaT`` when error is set to coerce (:issue:`26122`) | |||
pandas/_libs/tslib.pyx
Outdated
| @@ -757,7 +758,8 @@ cdef inline ignore_errors_out_of_bounds_fallback(ndarray[object] values): | |||
|
|
|||
| @cython.wraparound(False) | |||
| @cython.boundscheck(False) | |||
| cdef array_to_datetime_object(ndarray[object] values, bint is_raise, | |||
| cdef array_to_datetime_object(ndarray[object] values, | |||
There was a problem hiding this comment.
no, we already consistency pass around errors=, I think better here rather than having 2 args that are passed that could actually be in conflict.
| @@ -762,6 +762,18 @@ def test_iso_8601_strings_with_different_offsets(self): | |||
| NaT], tz='UTC') | |||
| tm.assert_index_equal(result, expected) | |||
|
|
|||
| # GH 26122 | |||
There was a problem hiding this comment.
can you make a new test that is named appropriately
|
thanks @makbigc very nice! keep em coming! |
git diff upstream/master -u -- "*.py" | flake8 --diff