Skip to content

Datetime error message (unable to continue that PR) #30711

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
Closed
wants to merge 8 commits into from
Closed

Datetime error message (unable to continue that PR) #30711

wants to merge 8 commits into from

Conversation

baevpetr
Copy link
Contributor

@baevpetr baevpetr commented Jan 5, 2020 •
edited
Loading

@gfyoung gfyoung added Error Reporting Incorrect or improved errors from pandas Timedelta Timedelta data type Datetime Datetime data dtype labels Jan 5, 2020
WillAyd
WillAyd requested changes Jan 6, 2020
View reviewed changes
pandas/core/arrays/datetimes.py Outdated
# GH#10720. If we failed to parse datetime then notify
# that flag errors='coerce' could be used to NaT.
# Trying to distinguish exception based on message.
if "Unknown string format" in e.args[0]:
Copy link
Member

Choose a reason for hiding this comment

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

Can you use raise from instead? I think makes things clearer with Python3

pandas/core/arrays/datetimes.py Outdated
if "Unknown string format" in e.args[0]:
msg = (
" ".join(e.args)
+ ". You can coerce to NaT by passing errors='coerce'"
Copy link
Member

Choose a reason for hiding this comment

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

Let's opt for f-strings over adding them like this

pandas/tests/indexes/datetimes/test_tools.py Outdated
invalid_data = "Month 1, 1999"
expected_args = (
f"Unknown string format: {invalid_data}. "
f"You can coerce to NaT by passing errors='coerce'"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
f"You can coerce to NaT by passing errors='coerce'"
"You can coerce to NaT by passing errors='coerce'"

Don't need the f prefix if not parametrizing anything

@baevpetr
Copy link
Contributor Author

baevpetr commented Jan 7, 2020 •
edited
Loading

Tests passed locally, but failed in Ci.
On some platforms

  1. There's extra %s flag:
    AssertionError: Pattern "Unknown string format: Month 1, 1999. You can coerce to NaT by passing errors='coerce'" not found in "Unknown string format: %s Month 1, 1999. You can coerce to NaT by passing errors='coerce'"
  2. or date is missing at all:
    AssertionError: Pattern "Unknown string format: Month 1, 1999. You can coerce to NaT by passing errors='coerce'" not found in "Unknown string format. You can coerce to NaT by passing errors='coerce'"
    Trying to figure it out...

WillAyd
WillAyd requested changes Jan 7, 2020
View reviewed changes
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

Might help with the CI failure as well

pandas/core/arrays/datetimes.py Outdated
# GH#10720. If we failed to parse datetime then notify
# that flag errors='coerce' could be used to NaT.
# Trying to distinguish exception based on message.
msg = " ".join(e.args)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
msg = " ".join(e.args)
msg = str(e)

@baevpetr
Copy link
Contributor Author

baevpetr commented Jan 7, 2020

str(e) doesn't pass even locally. Output looks like:
"('Unknown string format:', 'Month 1, 1999'). You can coerce to NaT by passing errors='coerce'"
Tried to use list comps. Doesn't help too.

@baevpetr
Copy link
Contributor Author

ping @WillAyd

@baevpetr
Copy link
Contributor Author

ping №2: @jbrockmendel

@jreback
Copy link
Contributor

jreback commented Jan 17, 2020

@baevpetr ci is not passing

jbrockmendel
jbrockmendel reviewed Jan 17, 2020
View reviewed changes
pandas/core/tools/timedeltas.py Outdated
@@ -113,9 +113,10 @@ def _coerce_scalar_to_timedelta_type(r, unit="ns", errors="raise"):

try:
result = Timedelta(r, unit)
except ValueError:
except ValueError as e:
Copy link
Member

Choose a reason for hiding this comment

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

can you call this "err" instead of "e". i really dislike 1-letter variable names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

even here. ok!)

jbrockmendel
jbrockmendel reviewed Jan 17, 2020
View reviewed changes
pandas/tests/indexes/datetimes/test_tools.py
invalid_data = "Month 1, 1999"
expected_args = (
f"Unknown string format: {invalid_data}. "
"You can coerce to NaT by passing errors='coerce'"
Copy link
Member

Choose a reason for hiding this comment

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

it looks like the "invalid_data" isnt making it into the error message in some of the tests

@baevpetr baevpetr closed this Jan 18, 2020
@simonjayhawkins simonjayhawkins mentioned this pull request Jan 18, 2020
Closed
4 tasks
@baevpetr baevpetr changed the title Datetime error message Datetime error message (unable to continue that PR) Jan 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbrockmendel jbrockmendel jbrockmendel left review comments

@WillAyd WillAyd WillAyd requested changes

Assignees
No one assigned
Labels
Datetime Datetime data dtype Error Reporting Incorrect or improved errors from pandas Timedelta Timedelta data type
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DOC/ERR: better error message on unsuccessful datetime parsing
5 participants
@baevpetr @jreback @WillAyd @jbrockmendel @gfyoung