Skip to content

ENH: add iso-format support to to_timedelta (#21877) #21933

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jul 20, 2018
Merged

ENH: add iso-format support to to_timedelta (#21877) #21933

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8fc1502
ENH: add iso-format support to to_timedelta (#21877)
fjdiod Jul 16, 2018
b5362e5
add test for iso format timedelta in to_timedelta
fjdiod Jul 16, 2018
b8d28f9
add whatsnew entry
fjdiod Jul 17, 2018
f0887a8
fix typo
fjdiod Jul 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion pandas/_libs/tslibs/timedeltas.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,11 @@ cpdef convert_to_timedelta64(object ts, object unit):
ts = cast_from_unit(ts, unit)
ts = np.timedelta64(ts)
elif is_string_object(ts):
ts = np.timedelta64(parse_timedelta_string(ts))
if len(ts) > 0 and ts[0] == 'P':
ts = parse_iso_format_string(ts)
else:
ts = parse_timedelta_string(ts)
ts = np.timedelta64(ts)
elif hasattr(ts, 'delta'):
ts = np.timedelta64(delta_to_nanoseconds(ts), 'ns')

Expand Down
5 changes: 5 additions & 0 deletions pandas/tests/indexes/timedeltas/test_construction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ def test_constructor(self):
tm.assert_index_equal(TimedeltaIndex([400, 450, 1200], unit='ms'),
expected)

def test_constructor_iso(self):
expected = timedelta_range('1s', periods=9, freq='s')
durations = ['P0DT0H0M{}S'.format(i) for i in range(1, 10)]
tm.assert_index_equal(to_timedelta(durations), expected)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit but we have a pseudo-standard of doing tm.assert_index_equal(result, expected) so would be nice to assign to a variable called result in the line above and simply do the comparison on the last line

Copy link
Member

@gfyoung gfyoung Jul 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More importantly, you should reference the issue number in a comment beyond the function def line.

Same applies to all tests that you have added.


def test_constructor_coverage(self):
rng = timedelta_range('1 days', periods=10.5)
exp = timedelta_range('1 days', periods=10)
Expand Down
4 changes: 4 additions & 0 deletions pandas/tests/scalar/timedelta/test_timedelta.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,10 @@ def check(value):
assert tup.microseconds == 999
assert tup.nanoseconds == 0

def test_iso_convertion(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor typo of the word conversion if you care to fix up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I push a fix for it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go for it

expected = Timedelta(1, unit='s')
assert to_timedelta('P0DT0H0M1S') == expected

def test_nat_converters(self):
assert to_timedelta('nat', box=False).astype('int64') == iNaT
assert to_timedelta('nan', box=False).astype('int64') == iNaT
Expand Down