Skip to content

bpo-31942: Document optional support of start and stop attributes in Sequence.index method#4277

Merged
vstinner merged 5 commits into
python:masterfrom
nitishch:bpo-31942
Dec 12, 2017
Merged

bpo-31942: Document optional support of start and stop attributes in Sequence.index method#4277
vstinner merged 5 commits into
python:masterfrom
nitishch:bpo-31942

Conversation

@nitishch

@nitishch nitishch commented Nov 4, 2017

Copy link
Copy Markdown
Contributor

I changed the method docstring and also the html documentation to say that not all implementations are required to support start and stop attributes. Is a NEWS entry required for this?

Bug: https://bugs.python.org/issue31942

https://bugs.python.org/issue31942

@vstinner

vstinner commented Dec 8, 2017

Copy link
Copy Markdown
Member

@serhiy-storchaka: Would you mind to review this PR, since you worked on that topic and created https://bugs.python.org/issue31942 ?

Comment thread Lib/_collections_abc.py Outdated
Raises ValueError if the value is not present.

It is not required that all concrete implementations of
the Sequence class support start and stop attributes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rephrase it as:

Supporting start and stop optional arguments is optional, but recommended.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to 'Supporting start and stop arguments is optional, but recommended.". I removed the first 'optional' word because the senses of both 'optional' words is different and might confuse the reader and also the sense of first 'optional' is conveyed by the function signature anyway.

Comment thread Doc/library/stdtypes.rst Outdated
without copying any data and with the returned index being relative to
the start of the sequence rather than the start of the slice.
Not all implementations support passing the additional arguments *i* and *j*.
When supported, these arguments allow efficient searching of

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Not all implementations support passing the additional arguments i and j." seems redundant with "When supported".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Comment thread Doc/library/stdtypes.rst Outdated
the start of the sequence rather than the start of the slice.
Not all implementations support passing the additional arguments
*i* and *j*. These arguments allow efficient searching of
subsections of the sequence. Passing the extra arguments is roughly

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid reflowing the lines that are not changed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minimised the reflowing.

Comment thread Lib/_collections_abc.py Outdated

Supporting start and stop arguments is optional, but
recommended.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant empty line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it.

@vstinner vstinner merged commit 5ce0a2a into python:master Dec 12, 2017
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @nitishch for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 12, 2017
@bedevere-bot

Copy link
Copy Markdown

GH-4811 is a backport of this pull request to the 3.6 branch.

vstinner pushed a commit that referenced this pull request Dec 12, 2017
@nitishch nitishch deleted the bpo-31942 branch December 13, 2017 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants