gh-64376: Convert the time module to the Argument Clinic by ZackerySpytz · Pull Request #14311 · python/cpython

ZackerySpytz · 2019-06-22T19:53:04Z

Co-Authored-By: Nikolaus Rath [email protected]

https://bugs.python.org/issue20177

Issue: Derby #8: Convert 28 sites to Argument Clinic across 2 files #64376

Co-Authored-By: Nikolaus Rath <[email protected]>

ZackerySpytz · 2019-06-22T20:34:39Z

As mentioned on the BPO issue, it doesn't seem practical to use the Argument Clinic instead of the gettmarg() utility function.

The time_clock_gettime() function was not converted -- there's an ifdef involving the PyArg_ParseTuple() call on AIX.

Lib/test/test_gdb.py

jdemeyer

Needs #14330

jdemeyer · 2019-08-05T09:51:01Z

Needs #14795 instead.

Since pythonGH-15776, the test no longer uses time_gmtime

encukou · 2019-09-10T15:14:35Z

There are now (in current master) dedicated built-in functions to use for testing, which aren't very likely to gain Argument Clinic signatures. I suggest using _testcapi.meth_varargs, which returns (its module, tuple of arguments):

        import _testcapi
        meth = types.MethodType(_testcapi.meth_varargs, 32)
        self.assertEqual(meth(), (_testcapi, (32,)))

jdemeyer · 2019-09-10T15:29:00Z

There are now (in current master) dedicated built-in functions to use for testing, which aren't very likely to gain Argument Clinic signatures. I suggest using _testcapi.meth_varargs, which returns (its module, tuple of arguments):

This PR shouldn't change test_gdb.py at all. The only reason that it was changed in this PR was to fix a test failure because test_gdb.py was using gmtime().

encukou · 2019-09-10T15:34:06Z

It no longer changes test_gdb. My comment was about Lib/unittest/test/testmock/testhelpers.py.
Sorry for the confusion.

serhiy-storchaka · 2020-07-10T19:23:08Z

Modules/timemodule.c

+/*[clinic input]
+time.gmtime
+
+    seconds: object = NULL


Maybe use a converter?

Suggested change

seconds: object = NULL

seconds as when: object(converter="object_to_time_t", type="time_t") = None

Or introduce a named converter?

Note also that you can use None as a default value for better signature.

serhiy-storchaka · 2020-07-10T19:34:48Z

Modules/timemodule.c

+time.strptime
+
+    string: object
+    format: object = NULL


Suggested change

format: object = NULL

format: object(c_default="NULL") = "%a %b %d %H:%M:%S %Y"

serhiy-storchaka · 2020-07-10T19:35:43Z

Modules/timemodule.c

+/*[clinic end generated code: output=00d2a25afc00983a input=474fd69ff9188bcb]*/
 {
-    PyObject *module, *func, *result;
+    PyObject *mod, *func, *result;


Why rename module?

serhiy-storchaka · 2020-07-10T19:37:57Z

Modules/timemodule.c

+Process time for profiling as nanoseconds:
+
+sum of the kernel and user-space CPU time.


It is a single sentence. Either write it in one line or make separate sentences.

bedevere-bot · 2020-07-10T19:41:47Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

taleinat · 2020-10-03T09:47:46Z

Ping, @ZackerySpytz?

erlend-aasland · 2023-05-09T21:55:23Z

No response since 2020. In the three years since 2020, there has been a lot of changes to Modules/timemodule.c¹. I suggest to close this PR; if someone wants to pick this up, it will probably be easier to start from scratch, rather than rebasing this PR.

569 insertions(+), 231 deletions(-) ↩

bpo-20177: Convert the time module to the Argument Clinic

d64e9b8

Co-Authored-By: Nikolaus Rath <[email protected]>

ZackerySpytz requested review from abalkin and pganssle as code owners June 22, 2019 19:53

the-knights-who-say-ni added the CLA signed label Jun 22, 2019

bedevere-bot added the awaiting review label Jun 22, 2019

jdemeyer reviewed Jun 24, 2019

View reviewed changes

Lib/test/test_gdb.py Outdated Show resolved Hide resolved

jdemeyer requested changes Jun 24, 2019

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Jun 24, 2019

Don't change test_gdb

4de2494

Since pythonGH-15776, the test no longer uses time_gmtime

serhiy-storchaka requested changes Jul 10, 2020

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting core review labels Jul 10, 2020

serhiy-storchaka added the topic-argument-clinic label Apr 30, 2022

ezio-melotti removed the CLA signed label Jul 13, 2022

larryhastings mentioned this pull request Apr 10, 2022

Derby #8: Convert 28 sites to Argument Clinic across 2 files #64376

Closed

erlend-aasland added the pending The issue will be closed if no feedback is provided label May 2, 2023

erlend-aasland changed the title ~~bpo-20177: Convert the time module to the Argument Clinic~~ bpo-64376: Convert the time module to the Argument Clinic May 2, 2023

erlend-aasland changed the title ~~bpo-64376: Convert the time module to the Argument Clinic~~ gh-64376: Convert the time module to the Argument Clinic May 2, 2023

erlend-aasland closed this May 9, 2023

erlend-aasland removed the pending The issue will be closed if no feedback is provided label May 9, 2023

	seconds: object = NULL
	seconds as when: object(converter="object_to_time_t", type="time_t") = None

	format: object = NULL
	format: object(c_default="NULL") = "%a %b %d %H:%M:%S %Y"

		Process time for profiling as nanoseconds:

		sum of the kernel and user-space CPU time.

Uh oh!

Conversation

ZackerySpytz commented Jun 22, 2019 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ZackerySpytz commented Jun 22, 2019

Uh oh!

Uh oh!

jdemeyer left a comment

Choose a reason for hiding this comment

Uh oh!

jdemeyer commented Aug 5, 2019

Uh oh!

encukou commented Sep 10, 2019

Uh oh!

jdemeyer commented Sep 10, 2019

Uh oh!

encukou commented Sep 10, 2019

Uh oh!

serhiy-storchaka Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Jul 10, 2020

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Jul 10, 2020

Uh oh!

taleinat commented Oct 3, 2020

Uh oh!

erlend-aasland commented May 9, 2023

Footnotes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ZackerySpytz commented Jun 22, 2019 •

edited by bedevere-bot

Loading