Skip to content

Accepts integer/float string with units and raises when unit is ambiguous (2) #23025

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

Closed
wants to merge 50 commits into from
Closed

Accepts integer/float string with units and raises when unit is ambiguous (2) #23025

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
ab5f7ba
Added null context manager as do_not_raise for parametrization in pyt…
Oct 5, 2018
7a38ecd
Added test for pure integer strings and test for strings with/without…
Oct 5, 2018
a65a35a
Added entry to whatsnew.
Oct 5, 2018
57dea1d
Changed Timedelta to raise for strings without unit.
Oct 5, 2018
b8cf952
WIP refactoring.
Oct 7, 2018
8154859
Deprecation warning when we don't have units in string numbers.
Oct 7, 2018
a90a992
Fixed linting.
Oct 7, 2018
1184b6e
Added to_timedelta test.
Oct 7, 2018
5e28b49
scalar case in to_timedelta checks for strings.
Oct 7, 2018
fafe838
Updated test with more cases for to_timedelta.
Oct 7, 2018
3ca0dae
All tests passing. Implemented checks on parse_timedelta_string.
Oct 7, 2018
8105f9c
Updated tests to avoid creating timedeltas from strings without unit.
Oct 7, 2018
e90a18b
Updated more tests to avoid creating timedeltas from strings without …
Oct 7, 2018
e0a55a8
Updated more tests to avoid creating timedeltas from strings without …
Oct 7, 2018
827aebf
Fixed failing test.
Oct 7, 2018
9bff504
Removed litle whitespace.
Oct 8, 2018
b6c7d45
Type specified unit as object.
Oct 8, 2018
9ff6ade
Removed newline.
Oct 8, 2018
be93e4b
Moved default unit down to more core functions.
Oct 10, 2018
85f0103
Fixed linting.
Oct 10, 2018
673ad19
Merge remote-tracking branch 'upstream/master' into bugfix/string-tim…
Oct 23, 2018
eb7d3f8
Merge remote-tracking branch 'origin/bugfix/string-timedelta2' into b…
Oct 23, 2018
bb40f77
Merge remote-tracking branch 'upstream/master' into bugfix/string-tim…
Oct 24, 2018
2e331c1
Merge remote-tracking branch 'upstream/master' into bugfix/string-tim…
Oct 31, 2018
bada4a7
Minor typo.
Nov 5, 2018
8c0425a
Lint
Nov 5, 2018
493a14b
Using ValueError instead of Exception.
Nov 5, 2018
e525ec4
Merge remote-tracking branch 'upstream/master' into bugfix/string-tim…
Nov 5, 2018
448b68a
Catching TypeError as well.
Nov 5, 2018
bd173a9
LINT
Nov 5, 2018
d16c4c5
Merge remote-tracking branch 'upstream/master' into bugfix/string-tim…
Nov 6, 2018
1cfc729
Refactored tests, fixed little bug in array_to_timedelta64 and explai…
Nov 6, 2018
e559f06
Fixed unit as None (instead of ns), Fixed test.
Nov 6, 2018
b914950
LINT: Fixed long lines.
Nov 10, 2018
64a4440
LINT: Fixed long lines.
Nov 10, 2018
4d26474
LINT: Fixed indentation of closing bracket.
Nov 10, 2018
db41054
Merge remote-tracking branch 'upstream/master' into bugfix/string-tim…
Nov 26, 2018
2f53004
Added comment.
Nov 26, 2018
fb40c94
Merge remote-tracking branch 'upstream/master' into bugfix/string-tim…
Nov 26, 2018
f5e59ee
Casting unit to 'ns' if None.
Nov 26, 2018
e7266da
Updated documentation and issuing DeprecationWarning instead of excep…
Nov 26, 2018
b6a02df
Fixed linting.
Nov 26, 2018
694abe9
DOC: Reorganized whatsnew
Nov 27, 2018
4d86ae1
CLN: Minor refactoring.
Nov 27, 2018
40b09f4
Merge remote-tracking branch 'upstream/master' into bugfix/string-tim…
Nov 27, 2018
a9c4675
Merge remote-tracking branch 'upstream/master' into bugfix/string-tim…
Nov 27, 2018
95302de
Merge remote-tracking branch 'upstream/master' into bugfix/string-tim…
Dec 3, 2018
661fd70
Merge remote-tracking branch 'upstream/master' into bugfix/string-tim…
Jan 7, 2019
80a1161
Added default value to func declaration.
Jan 8, 2019
dbe8fae
Fixed bug in warning string.
Jan 8, 2019
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
5 changes: 2 additions & 3 deletions pandas/_libs/tslibs/timedeltas.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,10 +532,9 @@ cpdef inline object parse_timedelta_unit(object unit):
----------
unit : an unit string
"""
if unit is None:
return 'ns'
elif unit == 'M':
if unit is None or unit == 'M':
return unit
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbrockmendel just explaining some changes to previous code so you can review, here I am leaving unit untouched if None because I thought was better only to do "nanoseconds if None" in the functions in the very end of the chain (where it is impossible to proceed without having something for units), not in the intermediate ones like this one here. This is because None is a perfectly valid state (user did not provide units) and we want to preserve it as long as possible to ensure correct behaviour through the whole process.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes this is ok, can you add a comment


try:
return timedelta_abbrevs[unit.lower()]
except (KeyError, AttributeError):
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/scalar/timedelta/test_construction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ def test_td_constructor_value_error():
def test_string_with_unit(value, str_unit, unit, expectation):
with expectation:
val_str = "{}{}".format(value, str_unit)
expected_td = Timedelta(10, unit=unit)
expected_td = Timedelta(value, unit=unit)

assert Timedelta(val_str, unit=unit) == expected_td
assert pd.to_timedelta(val_str, unit=unit) == expected_td
Expand Down