Skip to content

BUG: misleading message when using to_datetime with format='%V %a' #50309

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 3 commits into from
Dec 19, 2022
Merged

BUG: misleading message when using to_datetime with format='%V %a' #50309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7c57883
factor out iso year/week/weekday check, improve error message and
Dec 17, 2022
8de49d3
add extra test
Dec 17, 2022
b7f7ca9
dedupe
Dec 17, 2022
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
1 change: 1 addition & 0 deletions doc/source/whatsnew/v2.0.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,7 @@ Datetimelike
- Bug in :func:`to_datetime` was raising ``ValueError`` when parsing :class:`Timestamp`, ``datetime.datetime``, ``datetime.date``, or ``np.datetime64`` objects when non-ISO8601 ``format`` was passed (:issue:`49298`, :issue:`50036`)
- Bug in :func:`to_datetime` was raising ``ValueError`` when parsing empty string and non-ISO8601 format was passed. Now, empty strings will be parsed as :class:`NaT`, for compatibility with how is done for ISO8601 formats (:issue:`50251`)
- Bug in :class:`Timestamp` was showing ``UserWarning``, which was not actionable by users, when parsing non-ISO8601 delimited date strings (:issue:`50232`)
- Bug in :func:`to_datetime` was showing misleading ``ValueError`` when parsing dates with format containing ISO week directive and ISO weekday directive (:issue:`50308`)
-

Timedelta
Expand Down
52 changes: 32 additions & 20 deletions pandas/_libs/tslibs/strptime.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,38 @@ def array_strptime(
raise ValueError("Cannot use '%W' or '%U' without day and year")
elif "%Z" in fmt and "%z" in fmt:
raise ValueError("Cannot parse both %Z and %z")
elif "%j" in fmt and "%G" in fmt:
raise ValueError("Day of the year directive '%j' is not "
"compatible with ISO year directive '%G'. "
"Use '%Y' instead.")
elif "%G" in fmt and (
"%V" not in fmt
or not (
"%A" in fmt
or "%a" in fmt
or "%w" in fmt
or "%u" in fmt
)
):
raise ValueError("ISO year directive '%G' must be used with "
"the ISO week directive '%V' and a weekday "
"directive '%A', '%a', '%w', or '%u'.")
elif "%V" in fmt and "%Y" in fmt:
raise ValueError("ISO week directive '%V' is incompatible with "
"the year directive '%Y'. Use the ISO year "
"'%G' instead.")
elif "%V" in fmt and (
"%G" not in fmt
or not (
"%A" in fmt
or "%a" in fmt
or "%w" in fmt
or "%u" in fmt
)
):
raise ValueError("ISO week directive '%V' must be used with "
"the ISO year directive '%G' and a weekday "
"directive '%A', '%a', '%w', or '%u'.")

global _TimeRE_cache, _regex_cache
with _cache_lock:
Expand Down Expand Up @@ -328,26 +360,6 @@ def array_strptime(
weekday = int(found_dict["u"])
weekday -= 1

# don't assume default values for ISO week/year
if iso_year != -1:
if iso_week == -1 or weekday == -1:
raise ValueError("ISO year directive '%G' must be used with "
"the ISO week directive '%V' and a weekday "
"directive '%A', '%a', '%w', or '%u'.")
if julian != -1:
raise ValueError("Day of the year directive '%j' is not "
"compatible with ISO year directive '%G'. "
"Use '%Y' instead.")
Comment on lines -338 to -340
Copy link
Member Author

Choose a reason for hiding this comment

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

this one was never tested to begin with - have added a test to cover it

elif year != -1 and week_of_year == -1 and iso_week != -1:
if weekday == -1:
raise ValueError("ISO week directive '%V' must be used with "
"the ISO year directive '%G' and a weekday "
"directive '%A', '%a', '%w', or '%u'.")
else:
raise ValueError("ISO week directive '%V' is incompatible with "
"the year directive '%Y'. Use the ISO year "
"'%G' instead.")

# If we know the wk of the year and what day of that wk, we can figure
# out the Julian day of the year.
if julian == -1 and weekday != -1:
Expand Down
42 changes: 39 additions & 3 deletions pandas/tests/tools/test_to_datetime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,8 +629,8 @@ def test_to_datetime_iso_week_year_format(self, s, _format, dt):
"msg, s, _format",
[
[
"ISO week directive '%V' must be used with the ISO year directive "
"'%G' and a weekday directive '%A', '%a', '%w', or '%u'.",
"ISO week directive '%V' is incompatible with the year directive "
"'%Y'. Use the ISO year '%G' instead.",
Comment on lines -632 to +633
Copy link
Member Author

Choose a reason for hiding this comment

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

I've put the "%Y and %V used together" check before the "%V used without both %G and weekday directive", as I think it produces a more useful error message in this case

"1999 50",
"%Y %V",
],
Expand Down Expand Up @@ -706,10 +706,46 @@ def test_to_datetime_iso_week_year_format(self, s, _format, dt):
"20",
"%V",
],
[
"ISO week directive '%V' must be used with the ISO year directive "
"'%G' and a weekday directive '%A', '%a', '%w', or '%u'.",
"1999 51 Sunday",
"%V %A",
],
[
"ISO week directive '%V' must be used with the ISO year directive "
"'%G' and a weekday directive '%A', '%a', '%w', or '%u'.",
"1999 51 Sun",
"%V %a",
],
[
"ISO week directive '%V' must be used with the ISO year directive "
"'%G' and a weekday directive '%A', '%a', '%w', or '%u'.",
"1999 51 1",
"%V %w",
],
[
"ISO week directive '%V' must be used with the ISO year directive "
"'%G' and a weekday directive '%A', '%a', '%w', or '%u'.",
"1999 51 1",
"%V %u",
],
[
"Day of the year directive '%j' is not compatible with ISO year "
"directive '%G'. Use '%Y' instead.",
"1999 50",
"%G %j",
],
[
"ISO week directive '%V' must be used with the ISO year directive "
"'%G' and a weekday directive '%A', '%a', '%w', or '%u'.",
"20 Monday",
"%V %A",
],
Comment on lines +739 to +744
Copy link
Member Author

@MarcoGorelli MarcoGorelli Dec 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

on upstream/main, this one would show "ISO week directive '%V' is incompatible with the year directive '%Y'", which is misleading because we didn't use "%Y"

],
)
def test_error_iso_week_year(self, msg, s, _format):
# See GH#16607
# See GH#16607, #50308
# This test checks for errors thrown when giving the wrong format
# However, as discussed on PR#25541, overriding the locale
# causes a different error to be thrown due to the format being
Expand Down