gh-155496: Use Argument Clinic for more functions of the time module - #155513
gh-155496: Use Argument Clinic for more functions of the time module#155513serhiy-storchaka wants to merge 5 commits into
Conversation
It needs a builtin function without a signature, but time.ctime() now has one.
Any builtin function can get a signature, therefore the test uses the function which exists for testing the lack of it.
StanFromIreland
left a comment
StanFromIreland
left a comment
There was a problem hiding this comment.
Please ensure that the parameters match what is documented.
| return PyFloat_FromDouble(d); | ||
| } | ||
| /*[clinic input] | ||
| time.time -> double |
There was a problem hiding this comment.
Clinic won't emit these type annotations?
There was a problem hiding this comment.
No, it is only for code generation.
| /*[clinic input] | ||
| time.sleep | ||
|
|
||
| seconds as timeout_obj: object |
There was a problem hiding this comment.
The documented parameter name is secs, and it is not positional only.
There was a problem hiding this comment.
It is named seconds in the current docstring, and it is positional-only (METH_O).
There was a problem hiding this comment.
Yes, but please make the documentation consistent.
There was a problem hiding this comment.
This is a separate documentation issue, which is not only about main. Should we change the docs or the docstrings? Should we explicitly document parameters as positional-only in docs? What other in the docs does not match the behavior?
There was a problem hiding this comment.
You're already changing the docstrings, so I suggest you make them match the documentation. It will make follow-ups simpler if they are consistent.
| /*[clinic input] | ||
| time.localtime | ||
|
|
||
| seconds as ot: object = None |
There was a problem hiding this comment.
Documented parameter is secs.
There was a problem hiding this comment.
It is seconds in the current docstring.
| /*[clinic input] | ||
| time.asctime | ||
|
|
||
| time_tuple as tup: object = None |
There was a problem hiding this comment.
This introduces a behaviour change, previously:
>>> import time
>>> time.asctime(None)
Traceback (most recent call last):
File "<python-input-1>", line 1, in <module>
time.asctime(None)
~~~~~~~~~~~~^^^^^^
TypeError: Tuple or struct_time argument requiredBut with this patch, None falls back to localtime():
>>> import time
>>> time.asctime(None)
'Fri Aug 14 09:55:16 2026'There was a problem hiding this comment.
Good catch. Other functions accepted None.
|
|
||
| Convert a time in seconds since the Epoch to a string in local time. | ||
|
|
||
| This is equivalent to asctime(localtime(seconds)). When the time tuple |
There was a problem hiding this comment.
This was in the current docstring. Guess it needs a correction.
| /*[clinic input] | ||
| time.mktime | ||
|
|
||
| time_tuple as tm_tuple: object |
There was a problem hiding this comment.
Documented parameter is t.
There was a problem hiding this comment.
It is simply tuple in the current docstring. And referred as a time tuple.
Restore the default of time.asctime() as NULL, so that passing None raises a TypeError as before, and fix the docstring of time.ctime() and the comment for parse_time_t_arg().
2 participants
Convert all functions of the
timemodule, exceptstrftime()andstrptime().The docstring of
strftime()is composed with theSTRFTIME_FORMAT_CODESmacro shared withstrptime(), which Argument Clinic cannot express, andstrptime()only forwards its arguments to_strptime._strptime_time().time(),monotonic(),perf_counter(),process_time()andthread_time()now use the "double" return converter, which makes the_PyFloat_FromPyTime()helper redundant.