Skip to content

gh-109798: Normalize _datetime and datetime error messages #127345

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 31 commits into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
764eb1b
Update error messages to be the same in datetime
donBarbos Nov 27, 2024
2bf419f
Add NEWS.d/next
donBarbos Nov 27, 2024
4363e9e
fixed syntax errors
donBarbos Nov 27, 2024
9da0dfc
Move Py_DECREF after PyErr_Format
donBarbos Nov 29, 2024
179423d
Add more info in message error in _pydatetime impl
donBarbos Nov 29, 2024
f691251
Update Modules/_datetimemodule.c
donBarbos Nov 29, 2024
d8973cf
Update Modules/_datetimemodule.c
donBarbos Nov 29, 2024
5eea62f
Update Modules/_datetimemodule.c
donBarbos Nov 29, 2024
79543cc
Update Modules/_datetimemodule.c for optimisation
donBarbos Nov 29, 2024
d174497
Revert last update Modules/_datetimemodule.c
donBarbos Nov 29, 2024
498c4ba
Update Modules/_datetimemodule.c
donBarbos Nov 29, 2024
216d0fe
Update Misc/NEWS.d message
donBarbos Nov 29, 2024
c409fec
Update Misc/NEWS.d message
donBarbos Nov 29, 2024
3f454f6
Update Misc/NEWS.d message
donBarbos Nov 29, 2024
a2b8f7a
Update Misc/NEWS.d message
donBarbos Nov 29, 2024
209c338
Update Lib/_pydatetime.py
donBarbos Nov 30, 2024
0777aa5
Update Misc/NEWS.d/next/Library/2024-11-27-23-29-05.gh-issue-109798.O…
donBarbos Nov 30, 2024
4d31d33
Update Lib/_pydatetime.py
donBarbos Nov 30, 2024
f05ebba
Update Lib/_pydatetime.py
donBarbos Nov 30, 2024
f840105
Update Lib/_pydatetime.py
donBarbos Nov 30, 2024
61c95a5
Update Lib/_pydatetime.py
donBarbos Nov 30, 2024
2ab77b3
Update Lib/_pydatetime.py
donBarbos Nov 30, 2024
b1e272a
Update Lib/_pydatetime.py
donBarbos Nov 30, 2024
7a35bd4
Update Lib/_pydatetime.py
donBarbos Nov 30, 2024
cfd18cb
Add tests
donBarbos Nov 30, 2024
2827514
Update _pydatetime.py
donBarbos Dec 1, 2024
cd3bdc1
Update _pydatetime.py
donBarbos Dec 1, 2024
9915dfe
Change but got to not
donBarbos Dec 1, 2024
1da5a3a
Correct line break
donBarbos Dec 1, 2024
610f067
Update 2024-11-27-23-29-05.gh-issue-109798.OPj1CT.rst
pganssle Feb 12, 2025
410e0ce
Merge branch 'main' into issue-109798
pganssle Feb 12, 2025
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
43 changes: 23 additions & 20 deletions Lib/_pydatetime.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,14 @@ def _days_in_month(year, month):

def _days_before_month(year, month):
"year, month -> number of days in year preceding first day of month."
assert 1 <= month <= 12, 'month must be in 1..12'
assert 1 <= month <= 12, f"month must be in 1..12, not {month}"
return _DAYS_BEFORE_MONTH[month] + (month > 2 and _is_leap(year))

def _ymd2ord(year, month, day):
"year, month, day -> ordinal, considering 01-Jan-0001 as day 1."
assert 1 <= month <= 12, 'month must be in 1..12'
assert 1 <= month <= 12, f"month must be in 1..12, not {month}"
dim = _days_in_month(year, month)
assert 1 <= day <= dim, ('day must be in 1..%d' % dim)
assert 1 <= day <= dim, f"day must be in 1..{dim}, not {day}"
return (_days_before_year(year) +
_days_before_month(year, month) +
day)
Expand Down Expand Up @@ -512,7 +512,7 @@ def _parse_isoformat_time(tstr):
def _isoweek_to_gregorian(year, week, day):
# Year is bounded this way because 9999-12-31 is (9999, 52, 5)
if not MINYEAR <= year <= MAXYEAR:
raise ValueError(f"Year is out of range: {year}")
raise ValueError(f"year must be in {MINYEAR}..{MAXYEAR}, not {year}")

if not 0 < week < 53:
out_of_range = True
Expand Down Expand Up @@ -545,7 +545,7 @@ def _isoweek_to_gregorian(year, week, day):
def _check_tzname(name):
if name is not None and not isinstance(name, str):
raise TypeError("tzinfo.tzname() must return None or string, "
"not '%s'" % type(name))
f"not {type(name).__name__!r}")

# name is the offset-producing method, "utcoffset" or "dst".
# offset is what it returned.
Expand All @@ -558,24 +558,24 @@ def _check_utc_offset(name, offset):
if offset is None:
return
if not isinstance(offset, timedelta):
raise TypeError("tzinfo.%s() must return None "
"or timedelta, not '%s'" % (name, type(offset)))
raise TypeError(f"tzinfo.{name}() must return None "
f"or timedelta, not {type(offset).__name__!r}")
if not -timedelta(1) < offset < timedelta(1):
raise ValueError("%s()=%s, must be strictly between "
"-timedelta(hours=24) and timedelta(hours=24)" %
(name, offset))
raise ValueError("offset must be a timedelta "
"strictly between -timedelta(hours=24) and "
f"timedelta(hours=24), not {offset!r}")

def _check_date_fields(year, month, day):
year = _index(year)
month = _index(month)
day = _index(day)
if not MINYEAR <= year <= MAXYEAR:
raise ValueError('year must be in %d..%d' % (MINYEAR, MAXYEAR), year)
raise ValueError(f"year must be in {MINYEAR}..{MAXYEAR}, not {year}")
if not 1 <= month <= 12:
raise ValueError('month must be in 1..12', month)
raise ValueError(f"month must be in 1..12, not {month}")
dim = _days_in_month(year, month)
if not 1 <= day <= dim:
raise ValueError('day must be in 1..%d' % dim, day)
raise ValueError(f"day must be in 1..{dim}, not {day}")
return year, month, day

def _check_time_fields(hour, minute, second, microsecond, fold):
Expand All @@ -584,20 +584,23 @@ def _check_time_fields(hour, minute, second, microsecond, fold):
second = _index(second)
microsecond = _index(microsecond)
if not 0 <= hour <= 23:
raise ValueError('hour must be in 0..23', hour)
raise ValueError(f"hour must be in 0..23, not {hour}")
if not 0 <= minute <= 59:
raise ValueError('minute must be in 0..59', minute)
raise ValueError(f"minute must be in 0..59, not {minute}")
if not 0 <= second <= 59:
raise ValueError('second must be in 0..59', second)
raise ValueError(f"second must be in 0..59, not {second}")
if not 0 <= microsecond <= 999999:
raise ValueError('microsecond must be in 0..999999', microsecond)
raise ValueError(f"microsecond must be in 0..999999, not {microsecond}")
if fold not in (0, 1):
raise ValueError('fold must be either 0 or 1', fold)
raise ValueError(f"fold must be either 0 or 1, not {fold}")
return hour, minute, second, microsecond, fold

def _check_tzinfo_arg(tz):
if tz is not None and not isinstance(tz, tzinfo):
raise TypeError("tzinfo argument must be None or of a tzinfo subclass")
raise TypeError(
"tzinfo argument must be None or of a tzinfo subclass, "
f"not {type(tz).__name__!r}"
)

def _divide_and_round(a, b):
"""divide a by b and round result to the nearest integer
Expand Down Expand Up @@ -2418,7 +2421,7 @@ def __new__(cls, offset, name=_Omitted):
if not cls._minoffset <= offset <= cls._maxoffset:
raise ValueError("offset must be a timedelta "
"strictly between -timedelta(hours=24) and "
"timedelta(hours=24).")
f"timedelta(hours=24), not {offset!r}")
Copy link
Member

Choose a reason for hiding this comment

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

Probably not going to be a great user experience here because of how bad the timedelta formatter is:

>>> print(f"{timedelta(hours=-25)!r}")
datetime.timedelta(days=-2, seconds=82800)

Maybe it will be clearer if we do it this way:

if offset < timedelta(0):
    offset_str = f"-{-offset)}"
else:
     offset_str = str(offset)

That will print stuff like -1 day, 1:00:00 instead of timedelta(days=-2, seconds=82800). Though looking at it, I realize that the way it gets printed is misleading, since that's the output you get from print(timedelta(hours=-23)) and users have no way of knowing what is going on under the hood there.

Probably a more elaborate timedelta formatter would be better, since we basically always want this in HH:MM:SS.fff format, since that reflects what we care about best and also is unambigous, but we don't have a particularly easy way to do that. I guess we can revisit this when/if #85426 is implemented.

Copy link
Contributor Author

@donBarbos donBarbos Feb 11, 2025

Choose a reason for hiding this comment

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

@pganssle
Okay, then let's solve this issue in a separate PR linked to that issue (I can send the changes soon).
I think this one can be merged for now and I would be glad if you review another PR #127242 for datetime module :)

return cls._create(offset, name)

def __init_subclass__(cls):
Expand Down
69 changes: 69 additions & 0 deletions Lib/test/datetimetester.py
Original file line number Diff line number Diff line change
Expand Up @@ -1962,6 +1962,23 @@ def test_backdoor_resistance(self):
# blow up because other fields are insane.
self.theclass(base[:2] + bytes([ord_byte]) + base[3:])

def test_valuerror_messages(self):
pattern = re.compile(
r"(year|month|day) must be in \d+\.\.\d+, not \d+"
)
test_cases = [
(2009, 1, 32), # Day out of range
(2009, 2, 31), # Day out of range
(2009, 13, 1), # Month out of range
(2009, 0, 1), # Month out of range
(10000, 12, 31), # Year out of range
(0, 12, 31), # Year out of range
]
for case in test_cases:
with self.subTest(case):
with self.assertRaisesRegex(ValueError, pattern):
self.theclass(*case)

def test_fromisoformat(self):
# Test that isoformat() is reversible
base_dates = [
Expand Down Expand Up @@ -3212,6 +3229,24 @@ class DateTimeSubclass(self.theclass):
self.assertEqual(res.year, 2013)
self.assertEqual(res.fold, fold)

def test_valuerror_messages(self):
pattern = re.compile(
r"(year|month|day|hour|minute|second) must "
r"be in \d+\.\.\d+, not \d+"
)
test_cases = [
(2009, 4, 1, 12, 30, 90), # Second out of range
(2009, 4, 1, 12, 90, 45), # Minute out of range
(2009, 4, 1, 25, 30, 45), # Hour out of range
(2009, 4, 32, 24, 0, 0), # Day out of range
(2009, 13, 1, 24, 0, 0), # Month out of range
(9999, 12, 31, 24, 0, 0), # Year out of range
]
for case in test_cases:
with self.subTest(case):
with self.assertRaisesRegex(ValueError, pattern):
self.theclass(*case)

def test_fromisoformat_datetime(self):
# Test that isoformat() is reversible
base_dates = [
Expand Down Expand Up @@ -3505,6 +3540,25 @@ def test_fromisoformat_fails_datetime(self):
with self.assertRaises(ValueError):
self.theclass.fromisoformat(bad_str)

def test_fromisoformat_fails_datetime_valueerror(self):
pattern = re.compile(
r"(year|month|day|hour|minute|second) must "
r"be in \d+\.\.\d+, not \d+"
)
bad_strs = [
"2009-04-01T12:30:90", # Second out of range
"2009-04-01T12:90:45", # Minute out of range
"2009-04-01T25:30:45", # Hour out of range
"2009-04-32T24:00:00", # Day out of range
"2009-13-01T24:00:00", # Month out of range
"9999-12-31T24:00:00", # Year out of range
]

for bad_str in bad_strs:
with self.subTest(bad_str=bad_str):
with self.assertRaisesRegex(ValueError, pattern):
self.theclass.fromisoformat(bad_str)

def test_fromisoformat_fails_surrogate(self):
# Test that when fromisoformat() fails with a surrogate character as
# the separator, the error message contains the original string
Expand Down Expand Up @@ -4481,6 +4535,21 @@ def utcoffset(self, t):
t2 = t2.replace(tzinfo=Varies())
self.assertTrue(t1 < t2) # t1's offset counter still going up

def test_valuerror_messages(self):
pattern = re.compile(
r"(hour|minute|second|microsecond) must be in \d+\.\.\d+, not \d+"
)
test_cases = [
(12, 30, 90, 9999991), # Microsecond out of range
(12, 30, 90, 000000), # Second out of range
(25, 30, 45, 000000), # Hour out of range
(12, 90, 45, 000000), # Minute out of range
]
for case in test_cases:
with self.subTest(case):
with self.assertRaisesRegex(ValueError, pattern):
self.theclass(*case)

def test_fromisoformat(self):
time_examples = [
(0, 0, 0, 0),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added additional information into error messages in :mod:`datetime`, and made the messages more consistent between the C and Python implementations. Patch by Semyon Moroz.
51 changes: 25 additions & 26 deletions Modules/_datetimemodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -637,17 +637,19 @@ check_date_args(int year, int month, int day)
{

if (year < MINYEAR || year > MAXYEAR) {
PyErr_Format(PyExc_ValueError, "year %i is out of range", year);
PyErr_Format(PyExc_ValueError,
"year must be in %d..%d, not %d", MINYEAR, MAXYEAR, year);
return -1;
}
if (month < 1 || month > 12) {
PyErr_SetString(PyExc_ValueError,
"month must be in 1..12");
PyErr_Format(PyExc_ValueError,
"month must be in 1..12, not %d", month);
return -1;
}
if (day < 1 || day > days_in_month(year, month)) {
PyErr_SetString(PyExc_ValueError,
"day is out of range for month");
int dim = days_in_month(year, month);
if (day < 1 || day > dim) {
PyErr_Format(PyExc_ValueError,
"day must be in 1..%d, not %d", dim, day);
return -1;
}
return 0;
Expand All @@ -660,28 +662,25 @@ static int
check_time_args(int h, int m, int s, int us, int fold)
{
if (h < 0 || h > 23) {
PyErr_SetString(PyExc_ValueError,
"hour must be in 0..23");
PyErr_Format(PyExc_ValueError, "hour must be in 0..23, not %i", h);
return -1;
}
if (m < 0 || m > 59) {
PyErr_SetString(PyExc_ValueError,
"minute must be in 0..59");
PyErr_Format(PyExc_ValueError, "minute must be in 0..59, not %i", m);
return -1;
}
if (s < 0 || s > 59) {
PyErr_SetString(PyExc_ValueError,
"second must be in 0..59");
PyErr_Format(PyExc_ValueError, "second must be in 0..59, not %i", s);
return -1;
}
if (us < 0 || us > 999999) {
PyErr_SetString(PyExc_ValueError,
"microsecond must be in 0..999999");
PyErr_Format(PyExc_ValueError,
"microsecond must be in 0..999999, not %i", us);
return -1;
}
if (fold != 0 && fold != 1) {
PyErr_SetString(PyExc_ValueError,
"fold must be either 0 or 1");
PyErr_Format(PyExc_ValueError,
"fold must be either 0 or 1, not %i", fold);
return -1;
}
return 0;
Expand Down Expand Up @@ -1435,8 +1434,7 @@ new_timezone(PyObject *offset, PyObject *name)
GET_TD_DAYS(offset) < -1 || GET_TD_DAYS(offset) >= 1) {
PyErr_Format(PyExc_ValueError, "offset must be a timedelta"
" strictly between -timedelta(hours=24) and"
" timedelta(hours=24),"
" not %R.", offset);
" timedelta(hours=24), not %R", offset);
return NULL;
}

Expand Down Expand Up @@ -1505,10 +1503,10 @@ call_tzinfo_method(PyObject *tzinfo, const char *name, PyObject *tzinfoarg)
GET_TD_SECONDS(offset) == 0 &&
GET_TD_MICROSECONDS(offset) < 1) ||
GET_TD_DAYS(offset) < -1 || GET_TD_DAYS(offset) >= 1) {
Py_DECREF(offset);
PyErr_Format(PyExc_ValueError, "offset must be a timedelta"
" strictly between -timedelta(hours=24) and"
" timedelta(hours=24).");
" timedelta(hours=24), not %R", offset);
Py_DECREF(offset);
return NULL;
}
}
Expand Down Expand Up @@ -2261,7 +2259,7 @@ get_float_as_integer_ratio(PyObject *floatobj)
if (!PyTuple_Check(ratio)) {
PyErr_Format(PyExc_TypeError,
"unexpected return type from as_integer_ratio(): "
"expected tuple, got '%.200s'",
"expected tuple, not '%.200s'",
Py_TYPE(ratio)->tp_name);
Py_DECREF(ratio);
return NULL;
Expand Down Expand Up @@ -3382,7 +3380,8 @@ date_fromisocalendar(PyObject *cls, PyObject *args, PyObject *kw)
int rv = iso_to_ymd(year, week, day, &year, &month, &day);

if (rv == -4) {
PyErr_Format(PyExc_ValueError, "Year is out of range: %d", year);
PyErr_Format(PyExc_ValueError,
"year must be in %d..%d, not %d", MINYEAR, MAXYEAR, year);
return NULL;
}

Expand All @@ -3392,7 +3391,7 @@ date_fromisocalendar(PyObject *cls, PyObject *args, PyObject *kw)
}

if (rv == -3) {
PyErr_Format(PyExc_ValueError, "Invalid day: %d (range is [1, 7])",
PyErr_Format(PyExc_ValueError, "Invalid weekday: %d (range is [1, 7])",
day);
return NULL;
}
Expand Down Expand Up @@ -4378,8 +4377,7 @@ timezone_fromutc(PyDateTime_TimeZone *self, PyDateTime_DateTime *dt)
return NULL;
}
if (!HASTZINFO(dt) || dt->tzinfo != (PyObject *)self) {
PyErr_SetString(PyExc_ValueError, "fromutc: dt.tzinfo "
"is not self");
PyErr_SetString(PyExc_ValueError, "fromutc: dt.tzinfo is not self");
return NULL;
}

Expand Down Expand Up @@ -5352,7 +5350,8 @@ utc_to_seconds(int year, int month, int day,

/* ymd_to_ord() doesn't support year <= 0 */
if (year < MINYEAR || year > MAXYEAR) {
PyErr_Format(PyExc_ValueError, "year %i is out of range", year);
PyErr_Format(PyExc_ValueError,
"year must be in %d..%d, not %d", MINYEAR, MAXYEAR, year);
return -1;
}

Expand Down
Loading