factor out iso year/week/weekday check, improve error message and

MarcoGorelli · MarcoGorelli · commit b7b519331d9d · 2022-12-17T09:24:06.000Z
coverage
diff --git a/doc/source/whatsnew/v2.0.0.rst b/doc/source/whatsnew/v2.0.0.rst
@@ -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
diff --git a/pandas/_libs/tslibs/strptime.pyx b/pandas/_libs/tslibs/strptime.pyx
@@ -121,6 +121,40 @@ 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 "%G" in fmt and (
+            "%V" not in fmt
+            or not (
+                "%A" in fmt
+                or "%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 "%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 ("%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 "%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:
@@ -328,26 +362,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.")
-        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:
diff --git a/pandas/tests/tools/test_to_datetime.py b/pandas/tests/tools/test_to_datetime.py
@@ -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.",
                 "1999 50",
                 "%Y %V",
             ],
@@ -706,6 +706,30 @@ 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",
+            ],
         ],
     )
     def test_error_iso_week_year(self, msg, s, _format):

Original file line number	Diff line number	Diff line change
`@@ -786,6 +786,7 @@ Datetimelike`
`786`	`786`	- 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`)
`787`	`787`	- 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`)
`788`	`788`	- Bug in :class:`Timestamp` was showing ``UserWarning``, which was not actionable by users, when parsing non-ISO8601 delimited date strings (:issue:`50232`)
	`789`	+- Bug in :func:`to_datetime` was showing misleading ``ValueError`` when parsing dates with format containing ISO week directive and ISO weekday directive (:issue:`50308`)
`789`	`790`	`-`
`790`	`791`
`791`	`792`	`Timedelta`