Skip to content

ENH: Enable parsing of ISO8601-like timestamps with negative signs using pd.Timedelta (GH37172) #39497

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

Jump to bottom
Merged
merged 7 commits into from
Feb 10, 2021
Merged

ENH: Enable parsing of ISO8601-like timestamps with negative signs using pd.Timedelta (GH37172) #39497

merged 7 commits into from
Feb 10, 2021

Conversation

avinashpancham
Copy link
Contributor

jreback
jreback requested changes Feb 2, 2021
View reviewed changes
pandas/_libs/tslibs/timedeltas.pyx Outdated
@@ -275,7 +275,7 @@ cdef convert_to_timedelta64(object ts, str unit):
ts = cast_from_unit(ts, unit)
ts = np.timedelta64(ts, "ns")
elif isinstance(ts, str):
if len(ts) > 0 and ts[0] == "P":
if len(ts) > 0 and (ts[0] == "P" or ts[:2] == "-P"):
Copy link
Contributor

Choose a reason for hiding this comment

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

you need another length check here (e.g. if len(ts) ==1 this would raise)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

pandas/_libs/tslibs/timedeltas.pyx Outdated
@@ -746,11 +754,11 @@ cdef inline int64_t parse_iso_format_string(str ts) except? -1:
else:
raise ValueError(err_msg)

if not have_value:
if not valid_ts:
Copy link
Contributor

Choose a reason for hiding this comment

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

you cannot use have_value or maybe have_value && sign? adding this makes harder to understand

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have_value alone was sufficient

pandas/_libs/tslibs/timedeltas.pyx Outdated
@@ -1251,7 +1259,7 @@ class Timedelta(_Timedelta):
elif isinstance(value, str):
if unit is not None:
raise ValueError("unit must not be specified if the value is a str")
if len(value) > 0 and value[0] == 'P':
if len(value) > 0 and (value[0] == 'P' or value[:2] == "-P"):
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as above

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@jreback jreback added the Timedelta Timedelta data type label Feb 2, 2021
@pep8speaks
Copy link

pep8speaks commented Feb 5, 2021 •
edited
Loading

Hello @avinashpancham! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-02-05 22:04:18 UTC

@avinashpancham avinashpancham mentioned this pull request Feb 9, 2021
Closed
3 tasks
@jreback jreback added this to the 1.3 milestone Feb 10, 2021
@jreback jreback merged commit a92501e into pandas-dev:master Feb 10, 2021
@jreback
Copy link
Contributor

jreback commented Feb 10, 2021

thanks @avinashpancham

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Enhancement Timedelta Timedelta data type
Projects
None yet
Milestone
1.3
Development

Successfully merging this pull request may close these issues.

BUG: Handle negative sign (-) when parsing ISO 8601 durations
3 participants
@avinashpancham @pep8speaks @jreback