Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
22.01.14 07:13, Nikolaus Rath написав(ла): To me this version looks shorter and clearer. Is there really an advantage in defining the clinic argument as a custom struct rather than object? To me too. ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
Larry Hastings writes:
> All is not lost! What follows is rough pseudo-C code, hopefully you
> can take it from here.
>
>typedef struct {
> int set;
> time_t when;
>} clinic_time_t;
>
>#define DEFAULT_CLINIC_TIME_T {0, 0}
>
>static int
>parse_clinic_time_t_arg(PyObject *arg, clinic_time_t *ct)
>{
> if (arg == NULL)
> return 1;
> if (arg == Py_None)
> return 0;
> if (_PyTime_ObjectToTime_t(arg, &ct->when) == -1) {
> set = 1;
> return 0;
> }
> return 1;
>}
>
>static int post_parse_clinic_time_t(clinic_time_t *ct) {
> if (ct->set)
> return 0;
> ct->when = time(NULL);
> return 0;
>}
>
>/*[python input]
>class clinic_time_t_converter(CConverter):
> type = 'clinic_time_t'
> converter = 'parse_clinic_time_t'
> c_default = 'DEFAULT_CLINIC_TIME_T'
>[python start generated code]*/
>/*[python end generated code: checksum=...]*/
>
> Now you can use clinic_time_t. Parameters declared clinic_time_t can
> be required, or they can be optional; if they're optional give them a
> default value of None. You'll have to call post_parse_clinic_time_t()
> by hand in your impl function; I'll see if I can extend Clinic so
> converters can emit code after a successful call to the parse function
> but before the call to the impl.
Okay, I attached a patch along these lines to the bugtracker.
However, I have to say that I lost track of what we're actually gaining
with all this. If all we need is a type that can distinguish between a
valid time_t value and a default value, why don't we simply use
PyObject?
In other words, what's the advantage of all the code above over:
static int
PyObject_to_time_t(PyObject *obj, time_t *when)
{
if (obj == NULL || obj == Py_None)
*when = time(NULL);
else if (_PyTime_ObjectToTime_t(obj, when) == -1)
return 0;
return 1;
}
/*[clinic input]
time.gmtime
seconds: object=NULL
/
[...]
static PyObject *
time_gmtime_impl(PyModuleDef *module, PyObject *seconds)
{
PyObject *return_value = NULL;
time_t when;
if(!PyObj_to_time_t(seconds, &when))
return NULL;
[...]
To me this version looks shorter and clearer. Is there really an
advantage in defining the clinic argument as a custom struct rather than
object?
Best,
-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@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
Yes, I meant in the definition of the convertor class. You can fix c_default
in converter_init.
On Jan 21, 2014 7:19 PM, Nikolaus Rath wrote:
>
> Larry Hastings writes:
> > A comment on your approach so far: I'm very much against giving
> > "default" a default value in the constructor.
>
> You mean in the definition of the custom converter class?
>
> > I realize that hack saves you having to say "= NULL" in a lot of
> > places. But explicit is better than implicit, and we're going to read
> > these signatures a lot more often than we write them, and I want
> > Clinic signatures to be easy to read at first glance.
> []
> > All is not lost! What follows is rough pseudo-C code, hopefully you
> > can take it from here.
> >
> > typedef struct {
> > int set;
> > time_t when;
> > } clinic_time_t;
> > #define DEFAULT_CLINIC_TIME_T {0, 0}
> >
> [...]
> >
> > /*[python input]
> > class clinic_time_t_converter(CConverter):
> > type = 'clinic_time_t'
> > converter = 'parse_clinic_time_t'
> > c_default = 'DEFAULT_CLINIC_TIME_T'
> > [python start generated code]*/
> > /*[python end generated code: checksum=...]*/
> >
> > Now you can use clinic_time_t. Parameters declared clinic_time_t can
> > be required, or they can be optional; if they're optional give them a
> > default value of None.
>
> That doesn't work. If the default value is declared for the function
> rather than in the converter definition, it overwrites the C default:
>
> /*[clinic input]
> time.gmtime
>
> seconds: clinic_time_t=None
> /
> */
>
> gives:
>
> static PyObject *
> time_gmtime(PyModuleDef *module, PyObject *args)
> {
> PyObject *return_value = NULL;
> clinic_time_t seconds = Py_None;
>
> if (!PyArg_ParseTuple(args,
> "|O&:gmtime",
> parse_clinic_time_t, &seconds))
> goto exit;
> return_value = time_gmtime_impl(module, seconds);
>
> so the default for seconds is now Py_None instead of
> DEFAULT_CLINIC_TIME_T'.
>
> Best,
> 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@python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/larry%40hastings.org
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
Larry Hastings writes:
> A comment on your approach so far: I'm very much against giving
> "default" a default value in the constructor.
You mean in the definition of the custom converter class?
> I realize that hack saves you having to say "= NULL" in a lot of
> places. But explicit is better than implicit, and we're going to read
> these signatures a lot more often than we write them, and I want
> Clinic signatures to be easy to read at first glance.
[]
> All is not lost! What follows is rough pseudo-C code, hopefully you
> can take it from here.
>
>typedef struct {
> int set;
> time_t when;
>} clinic_time_t;
>#define DEFAULT_CLINIC_TIME_T {0, 0}
>
[...]
>
>/*[python input]
>class clinic_time_t_converter(CConverter):
> type = 'clinic_time_t'
> converter = 'parse_clinic_time_t'
> c_default = 'DEFAULT_CLINIC_TIME_T'
>[python start generated code]*/
>/*[python end generated code: checksum=...]*/
>
> Now you can use clinic_time_t. Parameters declared clinic_time_t can
> be required, or they can be optional; if they're optional give them a
> default value of None.
That doesn't work. If the default value is declared for the function
rather than in the converter definition, it overwrites the C default:
/*[clinic input]
time.gmtime
seconds: clinic_time_t=None
/
*/
gives:
static PyObject *
time_gmtime(PyModuleDef *module, PyObject *args)
{
PyObject *return_value = NULL;
clinic_time_t seconds = Py_None;
if (!PyArg_ParseTuple(args,
"|O&:gmtime",
parse_clinic_time_t, &seconds))
goto exit;
return_value = time_gmtime_impl(module, seconds);
so the default for seconds is now Py_None instead of
DEFAULT_CLINIC_TIME_T'.
Best,
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@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
On 01/21/2014 12:59 AM, Serhiy Storchaka wrote: 21.01.14 04:44, Nikolaus Rath написав(ла): Serhiy Storchaka writes: 20.01.14 06:19, Nikolaus Rath написав(ла): 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? /*[clinic input] time.gmtime [ seconds: time_t ] / Ahh, interesting. So this works, but now the C default is evaluated even if the user passed an argument (so that answers my question about decreased performance in the other subthread). The generated code is: Don't use time(NULL) as C default. Instead check group_right_1 and call time(NULL) explicitly. While this "trick" works, it abuses optional groups. Optional groups are intended as a last resort, for semantics that can't be expressed any other way. The semantics of time.gmtime() are very easily expressed using normal Python syntax. Please don't use optional groups here. //arry/ ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
21.01.14 04:44, Nikolaus Rath написав(ла):
Serhiy Storchaka writes:
20.01.14 06:19, Nikolaus Rath написав(ла):
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?
/*[clinic input]
time.gmtime
[
seconds: time_t
]
/
Ahh, interesting. So this works, but now the C default is evaluated even
if the user passed an argument (so that answers my question about
decreased performance in the other subthread). The generated code is:
Don't use time(NULL) as C default. Instead check group_right_1 and call
time(NULL) explicitly.
time_gmtime_impl(PyModuleDef *module, int group_right_1, time_t seconds)
{
if (!group_right_1)
seconds = time(NULL);
...
}
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
On 01/20/2014 06:44 PM, Nikolaus Rath wrote:
All in all, I'm still not sure how I'm supposed to proceed. I see the
following options (and I'm fine with all of them):
1. Use the option group with a custom converter. This means a time(NULL)
call even if the caller passed a parameter.
2. Declare the _impl parameter as PyObject* instead of time_t, and
explicitly call a C conversion function.
3. Patch clinic.py to only evaluate the C default if the caller does not
pass a parameter. This seemest cleanest, but I don't know if the
design of clinic.py actually allows that.
clinic.py is not flexible enough to allow initialization code after the
call to the converter.
A comment on your approach so far: I'm very much against giving
"default" a default value in the constructor. I realize that hack saves
you having to say "= NULL" in a lot of places. But explicit is better
than implicit, and we're going to read these signatures a lot more often
than we write them, and I want Clinic signatures to be easy to read at
first glance.
Anyway, you're right, the converter function is not called if a value is
not passed in to convert it. I think this is more complicated than you
suspect, because PyArg_ParseWhatnot doesn't tell you whether or not it
processed a parameter. You have to detect it yourself, generally
through a clever choice of a default value in C. But there are no
illegal values of time_t.
All is not lost! What follows is rough pseudo-C code, hopefully you can
take it from here.
typedef struct {
int set;
time_t when;
} clinic_time_t;
#define DEFAULT_CLINIC_TIME_T {0, 0}
static int
parse_clinic_time_t_arg(PyObject *arg, clinic_time_t *ct)
{
if (arg == NULL)
return 1;
if (arg == Py_None)
return 0;
if (_PyTime_ObjectToTime_t(arg, &ct->when) == -1) {
set = 1;
return 0;
}
return 1;
}
static int post_parse_clinic_time_t(clinic_time_t *ct) {
if (ct->set)
return 0;
ct->when = time(NULL);
return 0;
}
/*[python input]
class clinic_time_t_converter(CConverter):
type = 'clinic_time_t'
converter = 'parse_clinic_time_t'
c_default = 'DEFAULT_CLINIC_TIME_T'
[python start generated code]*/
/*[python end generated code: checksum=...]*/
Now you can use clinic_time_t. Parameters declared clinic_time_t can be
required, or they can be optional; if they're optional give them a
default value of None. You'll have to call post_parse_clinic_time_t()
by hand in your impl function; I'll see if I can extend Clinic so
converters can emit code after a successful call to the parse function
but before the call to the impl. Also, the converter probably isn't
quite right, you'll have to play with "impl_by_reference" and
"parse_by_reference" and add and remove asterisks and ampersands to make
sure the code is 100% correct. Examine the implementation of path_t in
Modules/posixmodule.c, as that does about the same thing.
And of course clinic_time_t is a poor name, perhaps you can come up with
a better one.
That should work,
//arry/
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
Serhiy Storchaka writes:
> 20.01.14 06:19, Nikolaus Rath написав(ла):
>> 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?
>
> /*[clinic input]
> time.gmtime
>
> [
> seconds: time_t
> ]
> /
>
Ahh, interesting. So this works, but now the C default is evaluated even
if the user passed an argument (so that answers my question about
decreased performance in the other subthread). The generated code is:
time_gmtime(PyModuleDef *module, PyObject *args)
{
PyObject *return_value = NULL;
int group_right_1 = 0;
time_t seconds = time(NULL);
switch (PyTuple_GET_SIZE(args)) {
case 0:
break;
case 1:
if (!PyArg_ParseTuple(args, "O&:gmtime", PyObject_to_time_t,
&seconds))
return NULL;
group_right_1 = 1;
break;
default:
PyErr_SetString(PyExc_TypeError, "time.gmtime requires 0 to 1
arguments");
return NULL;
}
return_value = time_gmtime_impl(module, group_right_1, seconds);
All in all, I'm still not sure how I'm supposed to proceed. I see the
following options (and I'm fine with all of them):
1. Use the option group with a custom converter. This means a time(NULL)
call even if the caller passed a parameter.
2. Declare the _impl parameter as PyObject* instead of time_t, and
explicitly call a C conversion function.
3. Patch clinic.py to only evaluate the C default if the caller does not
pass a parameter. This seemest cleanest, but I don't know if the
design of clinic.py actually allows that.
Best,
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@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
Ryan Smith-Roberts 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 wrote:
>
>> Larry Hastings 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@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.«
___
Python-Dev mailing list
Python-Dev@pytho
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
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. 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;
}
/*[clinic input]
time.gmtime
seconds: object = None
[clinic start generated code]*/
{
time_t when;
if (0 == parse_time_t_arg(seconds, &when))
return NULL;
...
[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 wrote:
> Larry Hastings 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@python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/rmsr%40lab.net
>
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
20.01.14 06:19, Nikolaus Rath написав(ла): 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? /*[clinic input] time.gmtime [ seconds: time_t ] / [clinic start generated code]*/ ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
Larry Hastings 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@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
Larry Hastings 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. I don't see how the conversion would result in more calls to time() than we have now. It seems to me that the expression for the C default should be only evaluated if the caller did not specify a value. Is that not how ac works? > 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? As long as you're comfortable looking over the (probably buggy) patch, yes, I'm happy to give it a shot. Best, 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@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
On Sun, Jan 19, 2014 at 2:38 AM, Larry Hastings wrote: > According to the issue tracker, " rmsr" has only ever filed one issue. > I just fixed (and closed) it. > The two issues were "custom converter with converter and default raises exception" and "custom converter with py_default and c_default being overridden by __init__". As for the former, you said "I hope you know what you're doing!" which made me step back and think more about the "why". I realized two things: The default-related class attributes might be an 'attractive nuisance', in that setting them there technically saves repetition, but could easily confuse a later reader who expects to find the defaults declared inline as usual. As well, it is unclear which of 'default', 'py_default', 'c_default' one needs to set, or which has priority. Nikolaus went ahead and set all three, thus my bug reports. After tinkering some more with the test file for the first bug, I noticed that a class with 'default' and 'converter' generates a default in the signature line but not at the C level. I'm wondering now if class-level default support shouldn't just be removed, as the aforementioned attractive nuisance. ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
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? As for why you're getting that exception, it definitely looks like a bug in Argument Clinic. I spotted another bug that would have bitten you while I was looking for this one, so I've opened bugs on both issues, and put you on the nosy list for them. According to the issue tracker, " rmsr" has only ever filed one issue. I just fixed (and closed) it. //arry/ ___ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
Ah yes, my apologies, I was thrown off by the first converter declaration
in your class and didn't spot the second, so didn't realize what you were
up to.
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).
As for why you're getting that exception, it definitely looks like a bug in
Argument Clinic. I spotted another bug that would have bitten you while I
was looking for this one, so I've opened bugs on both issues, and put you
on the nosy list for them.
On Sat, Jan 18, 2014 at 7:42 PM, Nikolaus Rath wrote:
> Hi Ryan,
>
>
> Ryan Smith-Roberts writes:
> > Hi Nikolaus. I also started a conversion of timemodule, but dropped it
> when
> > I saw in the issue that you had taken over that conversion. I also tried
> to
> > turn parse_time_t_args into a converter. However, it won't work. The
> > problem is that parse_time_t_args must be called whether or not the user
> > supplies an argument to the function, but an Argument Clinic converter
> only
> > gets called if the user actually supplies something, and not on the
> default
> > value.
>
> I don't quite follow. My approach was to drop parse_time_t_args()
> completely and use _PyTime_ObjectToTime_t() as the conversion function
> (which only needs to be called if the user supplied something).
>
> In other words, I would have expected
>
> >> ,
> >> | /*[python input]
> >> | class time_t_converter(CConverter):
> >> | type = 'time_t'
> >> | converter = 'time_t_converter'
> >> | default = None
> >> | py_default = 'None'
> >> | c_default = 'time(NULL)'
> >> | converter = '_PyTime_ObjectToTime_t'
> >> | [python start generated code]*/
> >> |
> >> | /*[clinic input]
> >> | time.localtime
> >> |
> >> | seconds: time_t
> >> | /
> >> |
> >> | bla.
> >> | [clinic start generated code]*/
> >> `
>
> to produce something like this:
>
> static PyObject *
> time_localtime(PyObject *self, PyObject *args)
> {
> PyObject *obj = NULL;
> time_t seconds;
> struct tm buf;
>
> if (!PyArg_ParseTuple(args, "|O:localtime", &obj))
> return NULL;
> if (obj == NULL || obj == Py_None)
> seconds = time(NULL);
> else {
> if (_PyTime_ObjectToTime_t(obj, &seconds) == -1)
> return NULL;
> }
> return time_localtime_impl(self, seconds);
> }
>
>
> Apart from getting an error from clinic.py, it seems to me that this
> should in principle be possible.
>
> Best,
> Nikolaus
>
>
> >
> > So, the best idea is to
> >
> > * Remove the PyArgs_ParseTuple code from parse_time_t_args
> > * Declare seconds as a plain object in Argument Clinic
> > * Call the modified parse_time_t_args on seconds first thing in the _impl
> > functions
> >
> >
> > On Sat, Jan 18, 2014 at 4:56 PM, Nikolaus Rath
> wrote:
> >
> >> Hello,
> >>
> >> I'm trying to convert functions using parse_time_t_args() (from
> >> timemodule.c) for argument parsing to argument clinic.
> >>
> >> The function is defined as:
> >>
> >> ,
> >> | static int
> >> | parse_time_t_args(PyObject *args, char *format, time_t *pwhen)
> >> | {
> >> | PyObject *ot = NULL;
> >> | time_t whent;
> >> |
> >> | if (!PyArg_ParseTuple(args, format, &ot))
> >> | return 0;
> >> | if (ot == NULL || ot == Py_None) {
> >> | whent = time(NULL);
> >> | }
> >> | else {
> >> | if (_PyTime_ObjectToTime_t(ot, &whent) == -1)
> >> | return 0;
> >> | }
> >> | *pwhen = whent;
> >> | return 1;
> >> | }
> >> `
> >>
> >> and used like this:
> >>
> >> ,
> >> | static PyObject *
> >> | time_localtime(PyObject *self, PyObject *args)
> >> | {
> >> | time_t when;
> >> | struct tm buf;
> >> |
> >> | if (!parse_time_t_args(args, "|O:localtime", &when))
> >> | return NULL;
> >> | if (pylocaltime(&when, &buf) == -1)
> >> | return NULL;
> >> | return tmtotuple(&buf);
> >> | }
> >> `
> >>
> >> In other words, if any Python object is passed to it, it calls
> >> _PyTime_ObjectToTime_t on it to convert it to time_t, and otherwise uses
> >> time(NULL) as the default value.
> >>
> >> May first attempt to implement something similar in argument clinic was:
> >>
> >> ,
> >> | /*[python input]
> >> | class time_t_converter(CConverter):
> >> | type = 'time_t'
> >> | converter = 'time_t_converter'
> >> | default = None
> >> | py_default = 'None'
> >> | c_default = 'time(NULL)'
> >> | converter = '_PyTime_ObjectToTime_t'
> >> | [python start generated code]*/
> >> |
> >> | /*[clinic input]
> >> | time.localtime
> >> |
> >> | seconds: time_t
> >> | /
> >> |
> >> | bla.
> >> | [clinic start generated code]*/
> >> `
> >>
> >> However, running clinic.py on this file gives:
> >>
> >> ,
> >> | $ Tools/clinic/clinic.py Modul
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
Hi Ryan,
Ryan Smith-Roberts writes:
> Hi Nikolaus. I also started a conversion of timemodule, but dropped it when
> I saw in the issue that you had taken over that conversion. I also tried to
> turn parse_time_t_args into a converter. However, it won't work. The
> problem is that parse_time_t_args must be called whether or not the user
> supplies an argument to the function, but an Argument Clinic converter only
> gets called if the user actually supplies something, and not on the default
> value.
I don't quite follow. My approach was to drop parse_time_t_args()
completely and use _PyTime_ObjectToTime_t() as the conversion function
(which only needs to be called if the user supplied something).
In other words, I would have expected
>> ,
>> | /*[python input]
>> | class time_t_converter(CConverter):
>> | type = 'time_t'
>> | converter = 'time_t_converter'
>> | default = None
>> | py_default = 'None'
>> | c_default = 'time(NULL)'
>> | converter = '_PyTime_ObjectToTime_t'
>> | [python start generated code]*/
>> |
>> | /*[clinic input]
>> | time.localtime
>> |
>> | seconds: time_t
>> | /
>> |
>> | bla.
>> | [clinic start generated code]*/
>> `
to produce something like this:
static PyObject *
time_localtime(PyObject *self, PyObject *args)
{
PyObject *obj = NULL;
time_t seconds;
struct tm buf;
if (!PyArg_ParseTuple(args, "|O:localtime", &obj))
return NULL;
if (obj == NULL || obj == Py_None)
seconds = time(NULL);
else {
if (_PyTime_ObjectToTime_t(obj, &seconds) == -1)
return NULL;
}
return time_localtime_impl(self, seconds);
}
Apart from getting an error from clinic.py, it seems to me that this
should in principle be possible.
Best,
Nikolaus
>
> So, the best idea is to
>
> * Remove the PyArgs_ParseTuple code from parse_time_t_args
> * Declare seconds as a plain object in Argument Clinic
> * Call the modified parse_time_t_args on seconds first thing in the _impl
> functions
>
>
> On Sat, Jan 18, 2014 at 4:56 PM, Nikolaus Rath wrote:
>
>> Hello,
>>
>> I'm trying to convert functions using parse_time_t_args() (from
>> timemodule.c) for argument parsing to argument clinic.
>>
>> The function is defined as:
>>
>> ,
>> | static int
>> | parse_time_t_args(PyObject *args, char *format, time_t *pwhen)
>> | {
>> | PyObject *ot = NULL;
>> | time_t whent;
>> |
>> | if (!PyArg_ParseTuple(args, format, &ot))
>> | return 0;
>> | if (ot == NULL || ot == Py_None) {
>> | whent = time(NULL);
>> | }
>> | else {
>> | if (_PyTime_ObjectToTime_t(ot, &whent) == -1)
>> | return 0;
>> | }
>> | *pwhen = whent;
>> | return 1;
>> | }
>> `
>>
>> and used like this:
>>
>> ,
>> | static PyObject *
>> | time_localtime(PyObject *self, PyObject *args)
>> | {
>> | time_t when;
>> | struct tm buf;
>> |
>> | if (!parse_time_t_args(args, "|O:localtime", &when))
>> | return NULL;
>> | if (pylocaltime(&when, &buf) == -1)
>> | return NULL;
>> | return tmtotuple(&buf);
>> | }
>> `
>>
>> In other words, if any Python object is passed to it, it calls
>> _PyTime_ObjectToTime_t on it to convert it to time_t, and otherwise uses
>> time(NULL) as the default value.
>>
>> May first attempt to implement something similar in argument clinic was:
>>
>> ,
>> | /*[python input]
>> | class time_t_converter(CConverter):
>> | type = 'time_t'
>> | converter = 'time_t_converter'
>> | default = None
>> | py_default = 'None'
>> | c_default = 'time(NULL)'
>> | converter = '_PyTime_ObjectToTime_t'
>> | [python start generated code]*/
>> |
>> | /*[clinic input]
>> | time.localtime
>> |
>> | seconds: time_t
>> | /
>> |
>> | bla.
>> | [clinic start generated code]*/
>> `
>>
>> However, running clinic.py on this file gives:
>>
>> ,
>> | $ Tools/clinic/clinic.py Modules/timemodule.c
>> | Error in file "Modules/timemodule.c" on line 529:
>> | Exception raised during parsing:
>> | Traceback (most recent call last):
>> | File "Tools/clinic/clinic.py", line 1445, in parse
>> | parser.parse(block)
>> | File "Tools/clinic/clinic.py", line 2738, in parse
>> | self.state(None)
>> | File "Tools/clinic/clinic.py", line 3468, in state_terminal
>> | self.function.docstring = self.format_docstring()
>> | File "Tools/clinic/clinic.py", line 3344, in format_docstring
>> | s += "".join(a)
>> | TypeError: sequence item 2: expected str instance, NoneType found
>> `
>>
>> What am I doing wrong?
>>
>>
>> Best,
>> 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@python.org
>> https://mail.python.org/mailman/listinfo/python-dev
>> Unsubscribe:
>
Re: [Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
Hi Nikolaus. I also started a conversion of timemodule, but dropped it when
I saw in the issue that you had taken over that conversion. I also tried to
turn parse_time_t_args into a converter. However, it won't work. The
problem is that parse_time_t_args must be called whether or not the user
supplies an argument to the function, but an Argument Clinic converter only
gets called if the user actually supplies something, and not on the default
value.
So, the best idea is to
* Remove the PyArgs_ParseTuple code from parse_time_t_args
* Declare seconds as a plain object in Argument Clinic
* Call the modified parse_time_t_args on seconds first thing in the _impl
functions
On Sat, Jan 18, 2014 at 4:56 PM, Nikolaus Rath wrote:
> Hello,
>
> I'm trying to convert functions using parse_time_t_args() (from
> timemodule.c) for argument parsing to argument clinic.
>
> The function is defined as:
>
> ,
> | static int
> | parse_time_t_args(PyObject *args, char *format, time_t *pwhen)
> | {
> | PyObject *ot = NULL;
> | time_t whent;
> |
> | if (!PyArg_ParseTuple(args, format, &ot))
> | return 0;
> | if (ot == NULL || ot == Py_None) {
> | whent = time(NULL);
> | }
> | else {
> | if (_PyTime_ObjectToTime_t(ot, &whent) == -1)
> | return 0;
> | }
> | *pwhen = whent;
> | return 1;
> | }
> `
>
> and used like this:
>
> ,
> | static PyObject *
> | time_localtime(PyObject *self, PyObject *args)
> | {
> | time_t when;
> | struct tm buf;
> |
> | if (!parse_time_t_args(args, "|O:localtime", &when))
> | return NULL;
> | if (pylocaltime(&when, &buf) == -1)
> | return NULL;
> | return tmtotuple(&buf);
> | }
> `
>
> In other words, if any Python object is passed to it, it calls
> _PyTime_ObjectToTime_t on it to convert it to time_t, and otherwise uses
> time(NULL) as the default value.
>
> May first attempt to implement something similar in argument clinic was:
>
> ,
> | /*[python input]
> | class time_t_converter(CConverter):
> | type = 'time_t'
> | converter = 'time_t_converter'
> | default = None
> | py_default = 'None'
> | c_default = 'time(NULL)'
> | converter = '_PyTime_ObjectToTime_t'
> | [python start generated code]*/
> |
> | /*[clinic input]
> | time.localtime
> |
> | seconds: time_t
> | /
> |
> | bla.
> | [clinic start generated code]*/
> `
>
> However, running clinic.py on this file gives:
>
> ,
> | $ Tools/clinic/clinic.py Modules/timemodule.c
> | Error in file "Modules/timemodule.c" on line 529:
> | Exception raised during parsing:
> | Traceback (most recent call last):
> | File "Tools/clinic/clinic.py", line 1445, in parse
> | parser.parse(block)
> | File "Tools/clinic/clinic.py", line 2738, in parse
> | self.state(None)
> | File "Tools/clinic/clinic.py", line 3468, in state_terminal
> | self.function.docstring = self.format_docstring()
> | File "Tools/clinic/clinic.py", line 3344, in format_docstring
> | s += "".join(a)
> | TypeError: sequence item 2: expected str instance, NoneType found
> `
>
> What am I doing wrong?
>
>
> Best,
> 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@python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/rmsr%40lab.net
>
___
Python-Dev mailing list
Python-Dev@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
[Python-Dev] Using argument clinic to replace timemodule.c:parse_time_t_args()
Hello,
I'm trying to convert functions using parse_time_t_args() (from
timemodule.c) for argument parsing to argument clinic.
The function is defined as:
,
| static int
| parse_time_t_args(PyObject *args, char *format, time_t *pwhen)
| {
| PyObject *ot = NULL;
| time_t whent;
|
| if (!PyArg_ParseTuple(args, format, &ot))
| return 0;
| if (ot == NULL || ot == Py_None) {
| whent = time(NULL);
| }
| else {
| if (_PyTime_ObjectToTime_t(ot, &whent) == -1)
| return 0;
| }
| *pwhen = whent;
| return 1;
| }
`
and used like this:
,
| static PyObject *
| time_localtime(PyObject *self, PyObject *args)
| {
| time_t when;
| struct tm buf;
|
| if (!parse_time_t_args(args, "|O:localtime", &when))
| return NULL;
| if (pylocaltime(&when, &buf) == -1)
| return NULL;
| return tmtotuple(&buf);
| }
`
In other words, if any Python object is passed to it, it calls
_PyTime_ObjectToTime_t on it to convert it to time_t, and otherwise uses
time(NULL) as the default value.
May first attempt to implement something similar in argument clinic was:
,
| /*[python input]
| class time_t_converter(CConverter):
| type = 'time_t'
| converter = 'time_t_converter'
| default = None
| py_default = 'None'
| c_default = 'time(NULL)'
| converter = '_PyTime_ObjectToTime_t'
| [python start generated code]*/
|
| /*[clinic input]
| time.localtime
|
| seconds: time_t
| /
|
| bla.
| [clinic start generated code]*/
`
However, running clinic.py on this file gives:
,
| $ Tools/clinic/clinic.py Modules/timemodule.c
| Error in file "Modules/timemodule.c" on line 529:
| Exception raised during parsing:
| Traceback (most recent call last):
| File "Tools/clinic/clinic.py", line 1445, in parse
| parser.parse(block)
| File "Tools/clinic/clinic.py", line 2738, in parse
| self.state(None)
| File "Tools/clinic/clinic.py", line 3468, in state_terminal
| self.function.docstring = self.format_docstring()
| File "Tools/clinic/clinic.py", line 3344, in format_docstring
| s += "".join(a)
| TypeError: sequence item 2: expected str instance, NoneType found
`
What am I doing wrong?
Best,
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@python.org
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
