This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author vstinner
Recipients rhettinger, serhiy.storchaka, vstinner
Date 2017-01-19.22:50:37
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1484866238.12.0.125124474938.issue29327@psf.upfronthosting.co.za>
In-reply-to
Content
Raymond: "Please be careful with all of these AC changes.  They need to have tests.  They need to not change APIs.  They need to not introduce bugs."

The bug was not introduced by an Argument Clinic change.

I agree that switching to AC must not change the API.

I didn't look much at the "recent" AC changes (it started somethings like 2 years ago, no?), but it seems like the most common trap are default values of optional positional arguments. Sometimes, there are inconsistencies between .rst doc (in Doc/ directory), the docstring and the C implementation. The trap is when the default is documented as None, the C code uses NULL and passing None behaves differently...


Raymond: "The whole effort seems to be being pushed forward in a cavalier and aggressive manner. In this case, there was an API change and bugs introduced for basically zero benefit."

I converted a few functions to AC. I used regular reviews for that, since I noticed that it's easy to make mistakes. My hope is that the AC conversion will also help to fix inconsistencies.

The benefit of switching to AC is a better docstring and a correct signature for inspect.signature(func). Python 3.5:
---
sorted(...)
    sorted(iterable, key=None, reverse=False) --> new sorted list
---
versus Python 3.7
---
sorted(iterable, key=None, reverse=False)
    Return a new list containing all items from the iterable in ascending order.
    
    A custom key function can be supplied to customize the sort order, and the
    reverse flag can be set to request the result in descending order.
---

IHMO Python 3.7 docstring looks better. (Sadly, sorted() doesn't use AC yet, the "AC code" was genereted manually. AC should support **kwargs, issue #20291.)



I guess "cavalier" and "aggressive" is more related to my FASTCALL work. I'm waiting for reviews for major API changes. I agree that I pushed a lot of code without reviews when I considered that the change was safe and simple. It became hard to get a review, there are too few reviewers, especially on the C code.

The benefit of FASTCALL are better performances. I'm trying to provide benchmarks whenever possible, but since I modified dozens of functions, sometimes I didn't publish all benchmark results (sometimes even to not spam an issue).


Microbenchmark on sorted() on Python 3.7 compared to 3.5 (before FASTCALL):
---
haypo@smithers$ ./python -m perf timeit 'seq=list(range(10))' 'sorted(seq)' --compare-to=../3.5/python -v
Median +- std dev: [3.5] 1.07 us +- 0.06 us -> [3.7] 958 ns +- 15 ns: 1.12x faster (-11%)

haypo@smithers$ ./python -m perf timeit 'seq=list(range(10)); k=lambda x:x' 'sorted(seq, key=k)' --compare-to=../3.5/python -v
Median +- std dev: [3.5] 3.34 us +- 0.07 us -> [3.7] 2.66 us +- 0.05 us: 1.26x faster (-21%)
---

It's not easy nor interesting to measure the speedup of the changes limited to sorted(). Sadly, FASTCALL requires to modify a lot of code to show its full power. Otherwise, the gain is much smaller.


The latest major "API change" was made by you 14 years ago. Yes, I introduced a bug, and I'm sorry about that. Shit happens.

Let's be more constructive: to avoid bugs, the best is to get reviews. I have dozens of patches waiting for your review, so please come to help me to spot bugs before releases ;-)
History
Date User Action Args
2017-01-19 22:50:38vstinnersetrecipients: + vstinner, rhettinger, serhiy.storchaka
2017-01-19 22:50:38vstinnersetmessageid: <1484866238.12.0.125124474938.issue29327@psf.upfronthosting.co.za>
2017-01-19 22:50:38vstinnerlinkissue29327 messages
2017-01-19 22:50:37vstinnercreate