[Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()

Nikolaus Rath Nikolaus at rath.org
Tue Jan 21 03:35:40 CET 2014


Ryan Smith-Roberts <rmsr at lab.net> writes:
> The trick you're missing is that *any time* you have an optional argument
> with a custom converter[1], PyArg_ParseTuple *only* calls the converter
> function in the case that the user *actually supplies some value*. This is
> a basic property of an optional argument. Another property is that the
> c_default is evaluated *every time*, as it is set *before* the call to
> PyArg_ParseTuple. Are these the best ways to do things? Maybe not, but it's
> how they are.
>
> Please do not use a custom converter for this case. It can't work. 

Well, I thought I was following Larry's recommendation:

>>> A better choice would be to write a converter function in C, then use
>>> a custom converter that called it.  Nikolaus: Is that something you're
>>> comfortable doing?

..and I assumed he'd know best. Did I misunderstand that quote?

> Please do what I outlined earlier (untested, somewhat verbose code
> follows):
>
> static int
> parse_time_t_arg(PyObject *arg, time_t *when)
> {
>     if (arg == NULL || arg == Py_None) {
>         *when = time(NULL);
>         return 1;
>     }
>     if (_PyTime_ObjectToTime_t(arg, when) == -1)
>         return 0;
>     return 1;
> }

Ahm, this is exactly the code that I wrote (and you even quoted it
below), only with the identifiers renamed.

>> /*[clinic input]
> time.gmtime
>     seconds: object = None
> [clinic start generated code]*/
> {
>     time_t when;
>
>     if (0 == parse_time_t_arg(seconds, &when))
>         return NULL;


That's fine with me too. I'd just like Larry to sign off on it, because
as far as I know, he'll be the one to review my patch.


Best,
-Nikolaus

> [1] If you set a default value, or put it in brackets as Serhiy later
> recommends, it works the same.
>
>
> On Sun, Jan 19, 2014 at 8:19 PM, Nikolaus Rath <Nikolaus at rath.org> wrote:
>
>> Larry Hastings <larry at hastings.org> writes:
>> > On 01/18/2014 09:52 PM, Ryan Smith-Roberts wrote:
>> >>
>> >> I still advise you not to use this solution. time() is a system call
>> >> on many operating systems, and so it can be a heavier operation than
>> >> you'd think. Best to avoid it unless it's needed (on FreeBSD it
>> >> seems to add about 15% overhead to localtime(), for instance).
>> >>
>> >
>> > I agree.  Converting to Argument Clinic should not cause a performance
>> > regression.  Please don't add new calls to time() for the sake of
>> > making code more generic.
>> >
>> > A better choice would be to write a converter function in C, then use
>> > a custom converter that called it.  Nikolaus: Is that something you're
>> > comfortable doing?
>>
>> I think I'll need some help. I don't know how to handle the case where
>> the user is not passing anything.
>>
>> Here's my attempt:
>>
>> ,----
>> | /* C Converter for argument clinic
>> |    If obj is NULL or Py_None, return current time. Otherwise,
>> |    convert Python object to time_t.
>> | */
>> | static int
>> | PyObject_to_time_t(PyObject *obj, time_t *stamp)
>> | {
>> |     if (obj == NULL || obj == Py_None) {
>> |         *stamp = time(NULL);
>> |     }
>> |     else {
>> |         if (_PyTime_ObjectToTime_t(obj, stamp) == -1)
>> |             return 0;
>> |     }
>> |     return 1;
>> | }
>> |
>> | /*[python input]
>> | class time_t_converter(CConverter):
>> |     type = 'time_t'
>> |     converter = 'PyObject_to_time_t'
>> |     default = None
>> | [python start generated code]*/
>> | /*[python end generated code:
>> checksum=da39a3ee5e6b4b0d3255bfef95601890afd80709]*/
>> |
>> |
>> | /*[clinic input]
>> | time.gmtime
>> |
>> |     seconds: time_t
>> |     /
>> |
>> | [clinic start generated code]*/
>> `----
>>
>> but this results in the following code:
>>
>> ,----
>> | static PyObject *
>> | time_gmtime(PyModuleDef *module, PyObject *args)
>> | {
>> |     PyObject *return_value = NULL;
>> |     time_t seconds;
>> |
>> |     if (!PyArg_ParseTuple(args,
>> |         "|O&:gmtime",
>> |         PyObject_to_time_t, &seconds))
>> |         goto exit;
>> |     return_value = time_gmtime_impl(module, seconds);
>> |
>> | exit:
>> |     return return_value;
>> | }
>> `----
>>
>> This works if the user calls time.gmtime(None), but it fails for
>> time.gmtime(). It seems that in that case my C converter function is
>> never called.
>>
>> What's the trick that I'm missing?
>>
>>
>> Thanks!
>> -Nikolaus
>>
>> --
>> Encrypted emails preferred.  PGP fingerprint: 5B93 61F8 4EA2 E279 ABF6
>> 02CF A9AD B7F8 AE4E 425C
>>
>>              »Time flies like an arrow, fruit flies like a Banana.«
>> _______________________________________________
>> Python-Dev mailing list
>> Python-Dev at python.org
>> https://mail.python.org/mailman/listinfo/python-dev
>> Unsubscribe:
>> https://mail.python.org/mailman/options/python-dev/rmsr%40lab.net
>>


-- 
Encrypted emails preferred.
PGP fingerprint: 5B93 61F8 4EA2 E279 ABF6  02CF A9AD B7F8 AE4E 425C

             »Time flies like an arrow, fruit flies like a Banana.«


More information about the Python-Dev mailing list