Skip to content

BUG/API: Disallow unit if input to Timedelta and to_timedelta is/contains str #34634

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

Merged
merged 3 commits into from
Jun 8, 2020

Conversation

pdnm
Copy link
Contributor

@pdnm pdnm commented Jun 7, 2020

@WillAyd
Copy link
Member

WillAyd commented Jun 7, 2020

Can you add a test for the behavior? This is typically the first thing we look for in any PR

@WillAyd WillAyd added Datetime Datetime data dtype Timedelta Timedelta data type and removed Datetime Datetime data dtype labels Jun 7, 2020
@jreback
Copy link
Contributor

jreback commented Jun 8, 2020

this is a replacement for #34379?

@jreback jreback added the Error Reporting Incorrect or improved errors from pandas label Jun 8, 2020
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

@pdnm like this PR a lot better; just some small comments, ping on green.

@pdnm
Copy link
Contributor Author

pdnm commented Jun 8, 2020

@jreback Fixed. Thanks!


# Usually, we have all strings. If so, we hit the fast path.
# If this path fails, we try conversion a different way, and
# this is where all of the error handling will take place.
try:
for i in range(n):
if isinstance(values[i], str):
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be in the clause on L251

@pdnm pdnm force-pushed the timedelta-string branch from 9340416 to 35923c0 Compare June 8, 2020 17:37
@pdnm pdnm force-pushed the timedelta-string branch from 35923c0 to 572ac39 Compare June 8, 2020 18:10
@pdnm pdnm requested a review from jreback June 8, 2020 18:59
@jreback jreback added this to the 1.1 milestone Jun 8, 2020
@jreback jreback merged commit d3f686b into pandas-dev:master Jun 8, 2020
@jreback
Copy link
Contributor

jreback commented Jun 8, 2020

thanks @pdnm very nice patch! keep em coming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Error Reporting Incorrect or improved errors from pandas Timedelta Timedelta data type
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG/API: to_timedelta unit-argument ignored for string input
3 participants