[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Inada Naoki added the comment: New changeset 9e4f2f3a6b8ee995c365e86d976937c141d867f8 by Inada Naoki in branch 'master': bpo-20180: Use argument clinic for dict.pop() and dict.popitem() (GH-12792) https://github.com/python/cpython/commit/9e4f2f3a6b8ee995c365e86d976937c141d867f8 --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Change by Inada Naoki : -- pull_requests: +12721 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Serhiy Storchaka added the comment: New changeset 65ce60aef150776f884715b4315a10a0d6ae769e by Serhiy Storchaka in branch 'master': bpo-20180: Simplify char_converter in Argument Clinic. (GH-9828) https://github.com/python/cpython/commit/65ce60aef150776f884715b4315a10a0d6ae769e --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Change by Serhiy Storchaka : -- pull_requests: +9203 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Raymond Hettinger added the comment: > Now just the collections module is left from this group. > Raymond, what would you like done with it WRT AC conversion, > if at all? I would like to pass on the collections module. Having looked at the patch, I think the code is currently better as is.

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Now just the collections module is left from this group. Raymond, what would you like done with it WRT AC conversion, if at all? -- ___ Python tracker

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Raymond Hettinger added the comment: New changeset c4bccd3c7617018b1ce16f95840ffe1a890d44df by Raymond Hettinger (Tal Einat) in branch 'master': bpo-20180: convert most of itertoolsmodule.c to use Argument Clinic (GH-9164)

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Change by Tal Einat : -- pull_requests: +8607 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Raymond Hettinger added the comment: New changeset 3286ce4adee85c5ce8ab3ee3089f3cd44a017fd7 by Raymond Hettinger (Tal Einat) in branch 'master': bpo-20180: itertools.groupby Argument Clinic conversion (GH-4170) https://github.com/python/cpython/commit/3286ce4adee85c5ce8ab3ee3089f3cd44a017fd7

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Now that transmogrify is done, the only open question is about itertools and collections. 1. Should collections be converted? 2. How to proceed with itertools? There are two PRs ready, with the one just for groupby() approved, but not yet reviewed by Raymond

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: New changeset c929df3b96c8d7e7977e581431192be21cdafd19 by Tal Einat in branch 'master': bpo-20180: complete AC conversion of Objects/stringlib/transmogrify.h (GH-8039) https://github.com/python/cpython/commit/c929df3b96c8d7e7977e581431192be21cdafd19 --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: See updated PR for Objects/stringlib/transmogrify.h in GH-8039. -- versions: +Python 3.8 -Python 3.7 ___ Python tracker ___

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Change by Tal Einat : -- pull_requests: +7648 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

INADA Naoki added the comment: > IMO we should create new issues for AC conversion of the collections and > random modules (assuming they haven't been converted yet), and close this > issue. I agree with you. Let's finish derby. -- ___ Python

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: IMO we should create new issues for AC conversion of the collections and random modules (assuming they haven't been converted yet), and close this issue. -- ___ Python tracker

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: To enable progressing with this in smaller steps, I created PR 4170 which converts only itertools.groupby and the intertnal _grouper with minimal required changes. -- ___ Python tracker

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Change by Tal Einat : -- pull_requests: +4139 ___ Python tracker ___ ___

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: See PR 4117 on GitHub with the itertools AC conversion. -- ___ Python tracker ___

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Change by Tal Einat : -- pull_requests: +4087 stage: needs patch -> patch review ___ Python tracker ___

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Serhiy Storchaka added the comment: Yes, it is. Actually the #include should be before lists of methods and static PyTypeObject initializers. I think it is better to move all method lists and type initializers to the bottom of the file. --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: As far as I can tell, with the creation of a separate clinic/itertools.c.h, the typedefs etc. need to be moved to the top of the file so that the #include can come after the typedefs and before the rest of the code which references clinic

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Serhiy Storchaka added the comment: Keep islice() non-converted. For the r parameter of permutations() use "r: object = None". -- ___ Python tracker

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: What about islice? Does AC now support complex enough signatures to support it? If not, should I leave the comment as is? -- ___ Python tracker

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Serhiy Storchaka added the comment: All correct. The final patch should be in the form of GitHub PR. Don't specify the self parameter explicitly, just rename the corresponding variable. The rest of the patch LGTM. --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Id be happy to update the itertools patch. I haven't been following AC work for quite a while. I would expect this to entail: 1. applying the patch to the master branch 2. merging any conflicts 3. running the clinic.py script again 4. review

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Serhiy Storchaka added the comment: Seems Raymond have changed his mind about using Argument Clinic in itertools (msg302908). Tal, do you mind to update your itertools patch and create a pull request? -- ___ Python

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Martin Panter added the comment: For str.format_map(mapping), yes the parsing happens in Objects/stringlib/unicode_format.h, but I don’t see that as a big problem. Moving this back to “needs patch”, assuming it is okay to convert format_map(). Other than from that, there are just tricky

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Roundup Robot added the comment: New changeset 01b06ca45f64 by INADA Naoki in branch 'default': Issue #20180: forgot to update AC output. https://hg.python.org/cpython/rev/01b06ca45f64 -- ___ Python tracker

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

INADA Naoki added the comment: Uhhh! I'm sorry. -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Serhiy Storchaka added the comment: Seems you haven't updated generated AC files. -- ___ Python tracker ___

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

INADA Naoki added the comment: Thank you, Martin and Serhiy. As http://bugs.python.org/issue20180#msg247987 , remained module to converted is transmogrify.h. But docstrings of methods in transmogrify.h were moved to bytes_methods.c in issue26765. May I close this issue for now? --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Roundup Robot added the comment: New changeset 27dc9a1c061e by INADA Naoki in branch 'default': Issue #20180: convert unicode methods to AC. https://hg.python.org/cpython/rev/27dc9a1c061e -- nosy: +python-dev ___ Python tracker

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Serhiy Storchaka added the comment: LGTM again. -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Changes by INADA Naoki : Added file: http://bugs.python.org/file46297/unicodeobject.c.v7.patch ___ Python tracker ___

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

INADA Naoki added the comment: Serhiy Storchaka, would you review again? Merging unicodeobject.c.v5.patch into default branch was almost straightforward. But there was one place I should rewrite. Here is the part of diff -u v5.patch v6.patch -- is old docstring w/o AC +- is current docstring

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Unfortunately, I won't be able to review this due to lack of time. -- ___ Python tracker ___

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

INADA Naoki added the comment: transmogrify.h uses hack to share docstring. I can't find straightforward way. -- ___ Python tracker ___

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

INADA Naoki added the comment: Updated patch for unicodeobject. @taleinat, could you confirm it? -- nosy: +inada.naoki Added file: http://bugs.python.org/file46292/unicodeobject.c.v6.patch ___ Python tracker

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: I'll commit #3 (unicodeobject.c) when I first get the chance, which should be in a few days. I'll be happy to improve the rest as needed if someone can find the time to review them! -- ___ Python tracker

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Robert Collins added the comment: Ok, so will someone commit 3), or would you like me to do so? After that it sounds like we can move this back to patch review, since there will be nothing left ready for commit. -- ___ Python tracker

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Summary of the current state of this AC conversion derby group: 1) The collections, itertools and random modules should not be converted (at least for now). 2) The three example modules should not be converted (Modules/xxlimited.c, Modules/xxmodule.c and

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Robert Collins added the comment: So I'm confused. Is this applicable or not? Its in commit review, but there's discussion of politics here :(. -- nosy: +rbcollins status: open - pending ___ Python tracker rep...@bugs.python.org

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Serhiy Storchaka added the comment: For now almost all generated code are written in separate file, so it doesn't attract attention (if you believe Argument Clinic). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20180

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Stefan Krah added the comment: Sadly, for political reasons, it's best that we not convert collections, itertools, or random for now. Well, there are also technical reasons. For example, when reviewing a huge patch at the beginning of this year, the sections that touched AC took me 10

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Here's a new patch for Objects/unicodeobject.c with all of Serhiy's suggestions integrated. -- Added file: http://bugs.python.org/file39190/unicodeobject.c.v4.patch ___ Python tracker rep...@bugs.python.org

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Hurrah! And that seems to be it for this group, since collections, itertools and random are not to be converted at this point, as well as the Modules/xx*.c files, and the stringlib files probably require overly extensive changes for conversion. --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Serhiy Storchaka added the comment: The patch causes a lot of warnings like Objects/unicodeobject.c: In function ‘unicode_title_impl’: Objects/unicodeobject.c:10581:5: warning: passing argument 1 of ‘_PyUnicode_Ready’ from incompatible pointer type [enabled by default] if

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Version of patch with PyUnicodeObject * warnings fixed. -- Added file: http://bugs.python.org/file39191/unicodeobject.c.v5.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20180

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Serhiy Storchaka added the comment: unicodeobject.c.v5.patch LGTM. -- stage: needs patch - commit review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20180 ___

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Attaching new patch for unicodeobject.c, which supersedes the previous patches. I've changed the name of the parameter of str.join() to iterable to match the docs. (In the previous patch I had changed it from seq to iterable_of_strings.) I also fixed two minor

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Larry Hastings added the comment: Sadly, for political reasons, it's best that we not convert collections, itertools, or random for now. I'll update this issue if the situation changes. Sorry about that! -- ___ Python tracker

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Changes by Serhiy Storchaka storch...@gmail.com: -- components: +Argument Clinic ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20180 ___ ___

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Changes by Stefan Krah stefan-use...@bytereef.org: -- nosy: -skrah ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20180 ___ ___ Python-bugs-list

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

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/issue20180 ___

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Martin v. Löwis added the comment: For the unicodeobject changes, see Rietveld. In addition, it produces many cast errors from PyUnicodeObject* to PyObject*. Otherwise, it looks fine - please apply (after these fixes). -- nosy: +loewis ___ Python

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Attached is a complete AC conversion of itertools. This is a large conversion, so I was extra careful. I even went over the entire diff manually to check for any errors. Compilation runs without warnings and the entire test suite passes. Notes: * I didn't convert

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: And at that, I've finished the conversion of this entire Derby group! Woohoo! (except for itertools.repeat which is pending discussion) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue20180

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Attached nearly complete conversion of Modules/_collectionsmodule.c. I've converted all functions using PyArg_* functions, including __new__ methods, but not deque.__init__. I'm attaching a separate patch for deque.__init__. -- Added file:

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Attached a patch converting deque.__init__ in Modules/_collectionsmodule.c, which completes the conversion of this file. This patch depends Larry's first patch in issue20341 (Argument Clinic: add nullable ints). This functionality is used to simplify the existing

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Attached is a complete conversion of Modules/_randommodule.c. Three _random.Random methods were converted: seed, getrandbits and __new__. There's was an problem converting _random.Random.__new__. I overcame it cleanly and think this patch is good. Details about

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Attached complete conversion of Modules/xxlimited.c, Modules/xxmodule.c and Modules/xxsubtype.c. There are no tests for these and the code is not used by anything AFAIK, so I was extra careful and went over the diffs manually once everything was done. --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Stefan Krah added the comment: I think we should probably not convert xx*.c. They are for newcomers who will likely be confused by the additional information overload. -- nosy: +skrah ___ Python tracker rep...@bugs.python.org

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: I thought AC was how all new modules were supposed to be written. If so, the example modules should definitely use AC! Also note that once the generated code is in separate files, the code in the origin files will be simpler and clearer. --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Stefan Krah added the comment: The vast majority of C modules are in third-party packages. I doubt that external modules will all use AC. xxmodule.c is about explaining the basic structure of a C extension. AC adds nothing to this and obfuscates (IMO) what is going on. --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Thanks, Stefan. I hadn't realized that the xx files were targeted mainly a third-party developers. In this case, I can see the sense in not converting them. I only wish someone had said something *before* I went ahead and converted them... --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Stefan Krah added the comment: I only wish someone had said something *before* I went ahead and converted them... Yeah, sorry. It's also just my personal opinion. BTW, xxmodule did not compile and there was a warning for xxsubtype. I just mention it in case these are AC issues. --

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Larry Hastings added the comment: When AC moves from internal-only tool to supported tool we'll want to convert the xx stuff. But yeah I reckon it makes sense to not convert them yet. The list of files is really more here's the list of stuff to check out and convert if necessary. best

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Attaching a new patch for complete conversion of Objects/unicodeobject.c. This is to replace the previous patch for this file. This new patch also converts all other possible unicode methods, including those not using PyArg_* functions. This includes various methods

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Attaching patch for complete conversion of Objects/unicodeobject.c. Notes: * maketrans() was already converted * converting the lstrip, rstrip and strip methods required a small, non-trivial change to the code, since they used a common function for argument parsing

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: Attached patch for AC conversion of Objects/stringlib/transmogrify.h. Converting the functions in this file required changes to Objects/bytesobject.c and Objects/bytearrayobject.c. Those changes are in the patch as well. The conversion of those files is part of

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

Tal Einat added the comment: I'm attaching my conversion of deque.rotate and _count_elements in Modules/_collectionsmodule.c. There remain 3 other functions in that module to be converted. One is deque_init which returns int; Larry said (on IRC) to hold off with that for now. The other two

[issue20180] Derby #11: Convert 50 sites to Argument Clinic across 9 files

New submission from Larry Hastings: This issue is part of the Great Argument Clinic Conversion Derby, where we're trying to convert as much of Python 3.4 to use Argument Clinic as we can before Release Candidate 1 on January 19. This issue asks you to change the following bundle of files: