Repository navigation
bpo-28754: AC on bisect. - #177
JulienPalard wants to merge 1 commit into
Conversation
Sorry, something went wrong.
methane
left a comment
There was a problem hiding this comment.
LGTM.
But please wait approval from expert.
(@rhettinger is listed on http://cpython-devguide.readthedocs.io/experts.html)
Sorry, something went wrong.
| #include "Python.h" | ||
|
|
||
| static int | ||
| ssize_t_converter(PyObject *obj, void *ptr) |
There was a problem hiding this comment.
ssize_t_converter is not used for parsing arguments. It is used only in optional_ssize_t_converter. Wouldn't be better to inline it?
Sorry, something went wrong.
|
|
||
| a: object | ||
| x: object | ||
| lo: Py_ssize_t(c_default='0') = 0 |
There was a problem hiding this comment.
c_default='0' is redundant.
Sorry, something went wrong.
|
|
||
|
|
||
| /*[clinic input] | ||
| bisect.bisect_right |
There was a problem hiding this comment.
You can use the return converter for simplifying the code.
bisect.bisect_right -> Py_ssize_t
The body of bisect_bisect_right_impl() would be just:
return internal_bisect_right(a, x, lo, hi);
Sorry, something went wrong.
|
|
||
| def test_hi_backcompatbility(self): | ||
| for func, data, elem, expected in self.precomputedCases: | ||
| self.assertEqual(func(data, elem, 0, -1), |
There was a problem hiding this comment.
This doesn't check that passing -1 and None is the same as not specifying the hi argument. Compare results with func(data, elem).
Sorry, something went wrong.
|
I don't want the pure python API to change at all. It is a long standing API and there is nothing wrong with it. Also, passing it -1 here as a special value makes real code less readable. Further, the change slightly slows down the case where high is specified. In a high performance building block module, we shouldn't add in unnecessary checks. |
Sorry, something went wrong.
|
To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request. If/when the requested changes have been made, please leave a comment that says, I have made the requested changes; please review again. That will trigger a bot to flag this pull request as ready for a follow-up review. |
Sorry, something went wrong.
…ute() I missed the C-fame case, when I changed the frame-function ref counting in Stackless issue python#117.
[bpo-28754]