Skip to content
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

datetime error message is different between _pydatetime.py and _datetimemodule.c #109798

Open
mattip opened this issue Sep 24, 2023 · 4 comments
Open
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@mattip
Copy link
Contributor

mattip commented Sep 24, 2023

Bug report

Bug description:

import _datetime, _pydatetime
_datetime.date(1, 1, 50)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: day is out of range for month```

_pydatetime.date(1, 1, 50)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python3.12/_pydatetime.py", line 960, in __new__
    year, month, day = _check_date_fields(year, month, day)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/_pydatetime.py", line 535, in _check_date_fields
    raise ValueError('day must be in 1..%d' % dim, day)
ValueError: ('day must be in 1..31', 50)

The error message differs between the two implementations of datetime. This came up when testing PyPy, which uses the pure-python datetime implementation. xref conda-forge/rtoml-feedstock#1 (comment)

CPython versions tested on:

3.12

Operating systems tested on:

Linux

Linked PRs

@mattip mattip added the type-bug An unexpected behavior, bug, or error label Sep 24, 2023
@iritkatriel iritkatriel added the stdlib Python modules in the Lib dir label Nov 26, 2023
@donBarbos
Copy link

donBarbos commented Nov 25, 2024

@mattip So, you suggest to leave the option as in the _datetime implementation?

I can send a PR

@mattip
Copy link
Contributor Author

mattip commented Nov 27, 2024

I don't really have an opinion which error message is better, only that they be the same.

@donBarbos
Copy link

Ok, I will leave error message as in the c implementation

Firstly, it is better to output a string (not tuple), and in the python implementation, the second value in tuple is redundant.
Secondly, it seems to me that it is the main implementation.

@picnixz picnixz added extension-modules C modules in the Modules dir 3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Nov 27, 2024
@donBarbos
Copy link

donBarbos commented Nov 27, 2024

I also add more inconsistency in the error messages that I found in the code

1. Errors about date and time have inconsistent messages, for example problem with year field may raise next errors:

ValueError: ('year must be in 1..9999', {year})
ValueError: Year is out of range: {year}
ValueError: year {year} is out of range
ValueError: Year {year} is out of range

We need to make same message template for the fields year, month, day, hour, minute, second, microsecond and fold.

2. Method date.fromisocalendar may raise next errors depends on implementation:

ValueError: Invalid weekday: {day} (range is [1, 7])
ValueError: Invalid day: {day} (range is [1, 7])

3. Error with offset argument in timezone may raise next errors:

ValueError: offset must be a timedelta strictly between -timedelta(hours=24) and timedelta(hours=24), not datetime.timedelta(days=21).  # with dot
ValueError: offset must be a timedelta strictly between -timedelta(hours=24) and timedelta(hours=24), not datetime.timedelta(days=21)   # without dot
ValueError: offset must be a timedelta strictly between -timedelta(hours=24) and timedelta(hours=24).
ValueError: {name}()={offset}, must be a timedelta strictly between -timedelta(hours=24) and timedelta(hours=24)

4. Errors about type of arugment

for example if i call _pydatetime.datetime() and _datetime.datetime() with tz i get:

TypeError: tzinfo argument must be None or of a tzinfo subclass                  # in _pydatetime
TypeError: tzinfo argument must be None or of a tzinfo subclass, not type 'int'  # in _datetime

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 bugs and security fixes 3.13 bugs and security fixes 3.14 new features, bugs and security fixes extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
Development

No branches or pull requests

4 participants