bpo-27867: Add a porting guide for PySlice_GetIndicesEx().#1973
bpo-27867: Add a porting guide for PySlice_GetIndicesEx().#1973serhiy-storchaka merged 8 commits intopython:masterpython/cpython:masterfrom serhiy-storchaka:PySlice_GetIndicesEx-porting-guideserhiy-storchaka/cpython:PySlice_GetIndicesEx-porting-guideCopy head branch name to clipboard
Conversation
| Returns ``0`` on success and ``-1`` on error with exception set. | ||
|
|
||
| .. note:: | ||
| This function considered not safe for resizable sequences. Replace its invocation :: |
There was a problem hiding this comment.
Is there a typo? Maybe is considered? Also there is a whitespace before ::. I do not know if it is intentional, considering you apply the :: without a whitespace in the sentence below
There was a problem hiding this comment.
Thank you. Yes, this is a typo. A whitespace before :: is intentional.
cryvate
left a comment
There was a problem hiding this comment.
The manual change looks good to me (could improve still), the what's new blurb is a tad confusing.
| Returns ``0`` on success and ``-1`` on error with exception set. | ||
|
|
||
| .. note:: | ||
| This function is considered not safe for resizable sequences. Replace its invocation :: |
There was a problem hiding this comment.
I thought about "is considered not" v.s. "is not considered" for a while, but actually the former/current version as its semantics seem more appropriate
| // return error | ||
| } | ||
|
|
||
| with using functions :c:func:`PySlice_Unpack` and :c:func:`PySlice_AdjustIndices`:: |
There was a problem hiding this comment.
You want to link to these functions here? It makes the sentence a bit of a roller coaster.
Could it be possible to move the "using ..." to after the code block? so:
with
if (Py [...]
using functions :c: ...
There was a problem hiding this comment.
Or restructure it, so that the whole block is
"This function is considered not safe for resizable sequences. Its invocation should be replaced by a combination of :c:... and :c:... where
if (PySlice_Get[...]
is replaced by
if (PySlice_Unpack..."
There was a problem hiding this comment.
Thank you, I'll restructure it.
| Changes in the C API | ||
| -------------------- | ||
|
|
||
| * Function :c:func:`PySlice_GetIndicesEx` is considered not safe for |
There was a problem hiding this comment.
"Function" -> "The function"
|
|
||
| * Function :c:func:`PySlice_GetIndicesEx` is considered not safe for | ||
| resizable sequences. It takes the current length of the sequence, but | ||
| if the slice indices are not instances of :class:`int`, but objects that |
There was a problem hiding this comment.
"[...] instances of :class:int" seems like an unended thoought. The double but in a row ("but if ..", "but objects") is quite confusing. Consider splitting into 2 or more sentences?
|
Thank you @cryvate for your review. |
|
No problem @serhiy-storchaka . Really like what you've changed it to. |
|
|
||
| is replaced by :: | ||
|
|
||
| if (PySlice_Unpack(slice, length, &start, &stop, &step) < 0) { |
There was a problem hiding this comment.
Remove length from argument list.
https://bugs.python.org/issue27867