-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: Don't error in pd.to_timedelta when errors=ignore #13832
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 all commits
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 |
---|---|---|
|
@@ -3,7 +3,9 @@ | |
""" | ||
|
||
import numpy as np | ||
import pandas as pd | ||
import pandas.tslib as tslib | ||
|
||
from pandas.types.common import (_ensure_object, | ||
is_integer_dtype, | ||
is_timedelta64_dtype, | ||
|
@@ -64,37 +66,22 @@ def to_timedelta(arg, unit='ns', box=True, errors='raise', coerce=None): | |
""" | ||
unit = _validate_timedelta_unit(unit) | ||
|
||
def _convert_listlike(arg, box, unit, name=None): | ||
|
||
if isinstance(arg, (list, tuple)) or not hasattr(arg, 'dtype'): | ||
arg = np.array(list(arg), dtype='O') | ||
|
||
# these are shortcutable | ||
if is_timedelta64_dtype(arg): | ||
value = arg.astype('timedelta64[ns]') | ||
elif is_integer_dtype(arg): | ||
value = arg.astype('timedelta64[{0}]'.format( | ||
unit)).astype('timedelta64[ns]', copy=False) | ||
else: | ||
value = tslib.array_to_timedelta64(_ensure_object(arg), | ||
unit=unit, errors=errors) | ||
value = value.astype('timedelta64[ns]', copy=False) | ||
|
||
if box: | ||
from pandas import TimedeltaIndex | ||
value = TimedeltaIndex(value, unit='ns', name=name) | ||
return value | ||
if errors not in ('ignore', 'raise', 'coerce'): | ||
raise ValueError("errors must be one of 'ignore', " | ||
"'raise', or 'coerce'}") | ||
|
||
if arg is None: | ||
return arg | ||
elif isinstance(arg, ABCSeries): | ||
from pandas import Series | ||
values = _convert_listlike(arg._values, box=False, unit=unit) | ||
return Series(values, index=arg.index, name=arg.name, dtype='m8[ns]') | ||
values = _convert_listlike(arg._values, unit=unit, | ||
box=False, errors=errors) | ||
return Series(values, index=arg.index, name=arg.name) | ||
elif isinstance(arg, ABCIndexClass): | ||
return _convert_listlike(arg, box=box, unit=unit, name=arg.name) | ||
return _convert_listlike(arg, unit=unit, box=box, | ||
errors=errors, name=arg.name) | ||
elif is_list_like(arg) and getattr(arg, 'ndim', 1) == 1: | ||
return _convert_listlike(arg, box=box, unit=unit) | ||
return _convert_listlike(arg, unit=unit, box=box, errors=errors) | ||
elif getattr(arg, 'ndim', 1) > 1: | ||
raise TypeError('arg must be a string, timedelta, list, tuple, ' | ||
'1-d array, or Series') | ||
|
@@ -142,13 +129,55 @@ def _validate_timedelta_unit(arg): | |
|
||
|
||
def _coerce_scalar_to_timedelta_type(r, unit='ns', box=True, errors='raise'): | ||
""" | ||
convert strings to timedelta; coerce to Timedelta (if box), else | ||
np.timedelta64 | ||
""" | ||
"""Convert string 'r' to a timedelta object.""" | ||
|
||
try: | ||
result = tslib.convert_to_timedelta64(r, unit) | ||
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.
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. Not in my implementation and for reasons that I explain below. |
||
except ValueError: | ||
if errors == 'raise': | ||
raise | ||
elif errors == 'ignore': | ||
return r | ||
|
||
# coerce | ||
result = pd.NaT | ||
|
||
result = tslib.convert_to_timedelta(r, unit, errors) | ||
if box: | ||
result = tslib.Timedelta(result) | ||
|
||
return result | ||
|
||
|
||
def _convert_listlike(arg, unit='ns', box=True, errors='raise', name=None): | ||
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. IOW this is correct, this needs to be fixed in cython 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. IMO the current layout is not correct. The reason why the |
||
"""Convert a list of objects to a timedelta index object.""" | ||
|
||
if isinstance(arg, (list, tuple)) or not hasattr(arg, 'dtype'): | ||
arg = np.array(list(arg), dtype='O') | ||
|
||
# these are shortcut-able | ||
if is_timedelta64_dtype(arg): | ||
value = arg.astype('timedelta64[ns]') | ||
elif is_integer_dtype(arg): | ||
value = arg.astype('timedelta64[{0}]'.format( | ||
unit)).astype('timedelta64[ns]', copy=False) | ||
else: | ||
try: | ||
value = tslib.array_to_timedelta64(_ensure_object(arg), | ||
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. this needs to be handled in cython 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. Same explanation as above. |
||
unit=unit, errors=errors) | ||
value = value.astype('timedelta64[ns]', copy=False) | ||
except ValueError: | ||
if errors == 'ignore': | ||
return arg | ||
else: | ||
# This else-block accounts for the cases when errors='raise' | ||
# and errors='coerce'. If errors == 'raise', these errors | ||
# should be raised. If errors == 'coerce', we shouldn't | ||
# expect any errors to be raised, since all parsing errors | ||
# cause coercion to pd.NaT. However, if an error / bug is | ||
# introduced that causes an Exception to be raised, we would | ||
# like to surface it. | ||
raise | ||
|
||
if box: | ||
from pandas import TimedeltaIndex | ||
value = TimedeltaIndex(value, unit='ns', name=name) | ||
return value |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
from numpy cimport ndarray, int64_t | ||
|
||
cdef convert_to_tsobject(object, object, object, bint, bint) | ||
cdef convert_to_timedelta64(object, object, object) | ||
cpdef convert_to_timedelta64(object, object) | ||
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. NO, don't change this 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. I had to, otherwise, it wouldn't be accessible in the Python world. This is really inconsequential anyways if you read the docs here. |
||
cpdef object maybe_get_tz(object) | ||
cdef bint _is_utc(object) | ||
cdef bint _is_tzlocal(object) | ||
|
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.
is this tested?
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.
Absolutely. That was one of the first tests that I wrote.