[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Shantanu]

Change by Shantanu :


--
nosy: +hauntsaninja
nosy_count: 5.0 -> 6.0
pull_requests: +19505
pull_request: https://github.com/python/cpython/pull/20208

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Jeroen Demeyer]

Change by Jeroen Demeyer :


--
pull_requests: +14150
pull_request: https://github.com/python/cpython/pull/14330

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Zackery Spytz]

Zackery Spytz  added the comment:

I have created a PR for Modules/timemodule.c.

--
nosy: +ZackerySpytz
versions: +Python 3.9 -Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Zackery Spytz]

Change by Zackery Spytz :


--
pull_requests: +14133
pull_request: https://github.com/python/cpython/pull/14311

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Nikolaus Rath  added the comment:

Sorry, no. I have long lost context and interest in this.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

The modern workflow requires creating a PR on GitHub. Nikolaus, do you mind to 
convert your patch to a PR?

--
nosy: +serhiy.storchaka
versions: +Python 3.8 -Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Tim Hoffmann]

Change by Tim Hoffmann :


--
pull_requests: +8052
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Petr Viktorin]

Change by Petr Viktorin :


--
nosy: +petr.viktorin

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Tal Einat]

Change by Tal Einat :


--
nosy:  -taleinat

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Serhiy Storchaka]

Changes by Serhiy Storchaka storch...@gmail.com:


--
components: +Argument Clinic

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Larry Hastings]

Larry Hastings added the comment:

All the Derby patches should only go into trunk at this point.

--
versions: +Python 3.5 -Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Nikolaus Rath added the comment:

Tal, I was referring to this mail: 
https://mail.python.org/pipermail/python-dev/2014-January/132066.html

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Tal Einat]

Tal Einat added the comment:

Regarding the handling of time_t arguments which can be None, I agree that the 
second version (without custom convertors) is simpler and clearer while having 
no disadvantage that I can see.

I'd like to review the rest of the patches, but you mention changes to the AC 
policy regarding 3.4 and 3.5 and I can't find a reference to these changes or 
to the new policy.

--
nosy: +taleinat

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Stefan Krah]

Changes by Stefan Krah stefan-use...@bytereef.org:


--
nosy:  -skrah

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Nikolaus Rath added the comment:

I've attached updated patches to reflect the recent changes in the derby policy.

part1 is the part of the patch that is suitable for 3.4.

part2 are the changes that have to wait for 3.5 (mostly because of default 
values).

--
Added file: http://bugs.python.org/file33714/timemodule_part1_rev3.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Changes by Nikolaus Rath nikol...@rath.org:


Added file: http://bugs.python.org/file33716/timemodule_part1_rev3.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Changes by Nikolaus Rath nikol...@rath.org:


Removed file: http://bugs.python.org/file33714/timemodule_part1_rev3.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Changes by Nikolaus Rath nikol...@rath.org:


Added file: http://bugs.python.org/file33717/timemodule_part2_rev3.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Nikolaus Rath added the comment:

I'll wait with working on _datetimemodule.c until someone had a chance to look 
over my _timemodule.c patch. (I want to make sure I'm following the right 
procedure).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Nikolaus Rath added the comment:

(I already sent this to python-dev, but maybe it makes more sense to have these 
thoughts together with the patch)

After looking at the conversion of parse_time_t_args again, I think I lost 
track of what we're actually gaining with this procedure. 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 the extra custom strict, plus the 
custom C converter function, plus the custom converter python class 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.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Nikolaus Rath added the comment:

As discussed on devel, here's an updated patch for timemodule.c that
uses a custom C converter to handle the parse_time_t_args() utility
function. The only function I did not convert is mktime, because I did
not find a way to do so without duplicating the gettmarg() utility
function (which cannot be converted because it is also called by other
C functions).

You probably want to take a close look at what I did to the strptime
function. I didn't see a way to include the actual default value
(rather than None) in the signature without reconstructing the
argument tuple for calling the Python implementation.

As far as I can see, this completes the conversion for timemodule.c. I'll
work on _datetimemodule.c next, but I'm not sure I'll have time before
next weekend.

This is probably already planned, but since I haven't seen it in
Misc/NEWS yet: I think it would be a good idea to entry informing the
user about the conversion from strictly optional parameters to
parameters with default values. Eg.:

 * Issues xxx, yyy, zzz: many functions in the standard library now
   accept `None` for optional arguments. Previously, specifying `None`
   mostly resulted in a `TypeError`. Starting with this version,
   passing `None` is equivalent to not specfying the parameter.

--
Added file: http://bugs.python.org/file33607/timemodule_rev2.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Larry Hastings]

Larry Hastings added the comment:

That macro, STRFTIME_FORMAT_CODES, is only used in two functions.  If it were 
me I'd just copy and paste the text into both functions.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Nikolaus Rath added the comment:

Will do. Another question: what should I do with return annotations, e.g. the 
- dict part in

get_clock_info(name: str) - dict\n

Should I drop that part? In the clinic howto, I couldn't find anything about 
return annotations (only that parameter annotations are unsupported).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Nikolaus Rath added the comment:

One more question: what's a good way to find sites that need changes? Searching 
for PyArg_ finds both converted and unconverted functions...

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Larry Hastings]

Larry Hastings added the comment:

Yes, you should drop things that look like return annotations in the original 
docstring.

I don't have a magic formula for finding unchanged functions, sorry.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Nikolaus Rath added the comment:

Here is a patch for timemodule.c. I have converted everything except: 

* ctime() uses the gettmarg() function. Converting gettmarg() itself doesn't 
seem possible to me (because it's not Python callable), and duplicating the 
functionality in a single complicated clinic section just for ctime() did not 
seem reasonable either.

* gmtime(), localtime(), ctime() use a the parse_time_t_args() utility 
function. I have not yet figured out how to replace this with argument clinic 
(sent a mail to python-dev about that).

--
Added file: http://bugs.python.org/file33534/timemodule.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Changes by Nikolaus Rath nikol...@rath.org:


Removed file: http://bugs.python.org/file33521/issue20177.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Nikolaus Rath added the comment:

Hmm. After reading some of the threads on python-dev, I'm now confused if I did 
the conversion of optional arguments correctly.

Should something like strftime(format[, tuple]) be converted using optional 
groups, or should tuple become a parameter with a default value? (so far, I 
have done the latter).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Larry Hastings]

Larry Hastings added the comment:

Optional groups really are only for cases when that's the behavior
of the original code.  If the original function used a switch statement
examining the size of the tuple, then had different parsing functions based on 
that size, use optional groups.

But if the function can be expressed with normal positional parameters and 
default values, that's best.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Nikolaus Rath]

Nikolaus Rath added the comment:

I have started working on this (partial patch attached), but I am unsure how to 
handle cases where the docstring definition utilizes a macro. For example:

PyDoc_STRVAR(strftime_doc,
strftime(format[, tuple]) - string\n\
\n\
Convert a time tuple to a string according to a format specification.\n\
See the library reference manual for formatting codes. When the time tuple\n\
is not present, current time as returned by localtime() is used.\n\
\n STRFTIME_FORMAT_CODES);

I don't think STRFTIME_FORMAT_CODES will be expanded if I put it into the 
[clinic] comment section. 

Should I include the contents verbatim? Or do not convert these functions to 
argument clinic?

--
keywords: +patch
nosy: +Nikratio
Added file: http://bugs.python.org/file33521/issue20177.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue20177] Derby #8: Convert 28 sites to Argument Clinic across 2 files

[Larry Hastings]

Larry Hastings added the comment:

Absolutely.  decimal is a very special case.

Whoever takes over this issue: please ignore the part about _decimal.c.

--
title: Derby #8: Convert 52 sites to Argument Clinic across 3 files - Derby 
#8: Convert 28 sites to Argument Clinic across 2 files

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue20177
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com