Corrected link targets in collections.rst #1052

MSeifert04 · 2017-04-08T11:28:47Z

Fixes several link targets in library.collections.rst.

There are 3 remaining "invalid" links:

list.append, because there is no "target" for that method. There is something documented in tutorial/datastructures but these are not valid link targets.
defaultdict.get, because it's not explicitly documented. Should it link to dict.get?
OrderedDict.update, because it's not explicitly documented. Should it link to dict.update?

mention-bot · 2017-04-08T11:28:49Z

@MSeifert04, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rhettinger, @Yhg1s and @birkenfeld to be potential reviewers.

serhiy-storchaka

Mostly looks good, but I have some comments.

serhiy-storchaka · 2017-04-08T15:22:01Z

Doc/library/collections.rst

@@ -357,21 +357,22 @@ or subtracting from an empty counter.
      restrictions on its keys and values.  The values are intended to be numbers
      representing counts, but you *could* store anything in the value field.

-    * The :meth:`most_common` method requires only that the values be orderable.
+    * The :meth:`~Counter.most_common` method requires only that the values be


Change also :func:'most_common' above to :meth:'most_common'.

serhiy-storchaka · 2017-04-08T15:26:36Z

Doc/library/collections.rst

-the :meth:`rotate` method to position elements to be popped::
+The :meth:`~deque.rotate` method provides a way to implement :class:`deque` slicing
+and deletion.  For example, a pure Python implementation of ``del d[n]`` relies on
+the :meth:`~deque.rotate` method to position elements to be popped::


Avoid using the same link multiple times in one paragraph. ``rotate()`` would look better.

serhiy-storchaka · 2017-04-08T15:29:18Z

Doc/library/collections.rst

-   passed to the :class:`OrderedDict` constructor and its :meth:`update`
-   method.
+   passed to the :class:`OrderedDict` constructor and its
+   :meth:`update` method.


What is changed here?

Nothing.

I originally changed it to dict.update but then I wasn't sure if that was what was intended and reverted it again. Just forgot to revert all of the change.

serhiy-storchaka · 2017-04-08T15:30:59Z

Doc/library/collections.rst

@@ -1175,6 +1176,11 @@ attribute.
    subclass) or an arbitrary sequence which can be converted into a string using
    the built-in :func:`str` function.

+    .. attribute:: data


Shouldn't be added the paragraph "In addition to supporting ..." as for UserDict and UserList?

serhiy-storchaka · 2017-04-08T15:38:18Z

Doc/library/collections.rst

@@ -1175,6 +1176,11 @@ attribute.
    subclass) or an arbitrary sequence which can be converted into a string using


This phrase is outdated (remained from Python 2). Virtually any object (not just sequences) can be converted into a string using str(). However bytes is bad example, it emits a warning or an exception when converted to str.

vadmium · 2017-04-09T02:49:41Z

Doc/library/collections.rst

-    be an instance of :class:`bytes`, :class:`str`, :class:`UserString` (or a
-    subclass) or an arbitrary sequence which can be converted into a string using
-    the built-in :func:`str` function.
+    contents are initially set to a copy of *seq*.  The *seq* can


“The *seq*” is missing a noun. I would either write “*Seq* can . . .” or “The *seq* argument can . . .”. Changing the parameter name makes this problem worse.

orsenthil

LGTM.

serhiy-storchaka · 2017-04-10T16:21:26Z

@rhettinger could you please make a look as the collections module maintainer?

Mariatta · 2017-04-10T23:05:07Z

I think this looks good, I just wonder if we need a ticket in b.p.o for this?

rhettinger · 2017-04-11T03:18:22Z

Please hold off on this for while. I won't have a chance to go through every change in detail for a while. Some of the changes like func->meth or meth->attr look fine; however, a number of the meth entries were intentionally not made into link target (i.e. I mention rotate() several times in a row, I don't want them to all be links -- this just garbages the appearance without making the docs better. The double colon was left-off of the combination_with_replacement example on purpose because it includes sample output which isn't real code. That said, I think it looks better with the double colon, so I think I'll put a # comment on the line or somesuch. When proposing extensive edits, please don't rewrap the lines unless there is a major increase to line length, the rewrapping makes it harder to review.

MSeifert04 · 2017-04-12T17:13:32Z

I haven't realized that this was an "extensive edit". If it would help I can seperate the different changes into seperate PRs (with accompanying issues on the tracker).

There are basically 4 changes here:

wrong target :meth: instead of :attr: or :func: instead of :meth:
missing data attribute and outdated documentation of Userstring
the double colon when referencing itertools.combinations
prepending the class to "not-linking" links

MSeifert04 · 2017-05-12T13:56:12Z

How should I proceed with this PR?

MSeifert04 · 2017-09-07T20:34:23Z

Anything I can or should do?

It's a relatively small PR and it has been stalling for months now.

MSeifert04 · 2017-09-16T00:22:55Z

@rhettinger Is it worth fixing the conflicts? I don't want to invest more time into this PR if you don't like it.

MSeifert04 · 2017-09-23T00:02:17Z

@serhiy-storchaka Thanks for fixing the conflicts :)

I still wonder if it's worth keeping it open. There seems to be no interest in merging it, or is just that @rhettinger has no time to review it?

serhiy-storchaka · 2017-09-23T05:07:56Z

All @rhettinger's comments looks addressed to me.

MSeifert04 · 2017-09-23T15:13:25Z

There is one comment that I haven't addressed (I think) because I don't know which ones were meant:

however, a number of the meth entries were intentionally not made into link target (i.e. I mention rotate() several times in a row, I don't want them to all be links -- this just garbages the appearance without making the docs better.

serhiy-storchaka · 2017-09-23T16:44:42Z

It was addressed before it was written. There are no more than one links to the same target in one paragraph. If rotate() is mentioned several times in a row, only the first occurrence is a link.

MSeifert04 · 2017-09-25T16:10:16Z

okay - so what is holding up the PR?

MSeifert04 · 2017-10-07T16:33:10Z

@rhettinger Do you have time to review the PR?

miss-islington · 2018-03-26T11:40:37Z

Thanks @MSeifert04 for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

(cherry picked from commit e105294) Co-authored-by: Michael Seifert <michaelseifert04@yahoo.de>

bedevere-bot · 2018-03-26T11:41:45Z

GH-6257 is a backport of this pull request to the 3.7 branch.

miss-islington · 2018-03-26T11:42:40Z

Sorry, @MSeifert04 and @serhiy-storchaka, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker e105294708ffc78a31566b48468746eb4609abe7 3.6

(cherry picked from commit e105294) Co-authored-by: Michael Seifert <michaelseifert04@yahoo.de>

Corrected link targets in collections.rst

bc1cb5a

the-knights-who-say-ni added the CLA signed label Apr 8, 2017

serhiy-storchaka reviewed Apr 8, 2017

View reviewed changes

updated according to comments

6ae77ec

serhiy-storchaka approved these changes Apr 8, 2017

View reviewed changes

serhiy-storchaka requested a review from rhettinger April 8, 2017 16:05

vadmium reviewed Apr 9, 2017

View reviewed changes

fixed grammar

40ac948

orsenthil approved these changes Apr 9, 2017

View reviewed changes

orsenthil added needs backport to 3.5 labels Apr 9, 2017

rhettinger self-assigned this Apr 11, 2017

undo line breaks and comment "not-code" example

9b48be9

brettcannon removed the needs backport to 3.5 label Aug 11, 2017

Merge branch 'master' into fix_links_collection_docs

e7d12ef

serhiy-storchaka added skip news docs Documentation in the Doc dir labels Sep 16, 2017

brettcannon added the awaiting merge label Feb 21, 2018

serhiy-storchaka added the needs backport to 3.7 label Mar 26, 2018

Merge branch 'master' into fix_links_collection_docs

5957f98

serhiy-storchaka added the skip issue label Mar 26, 2018

serhiy-storchaka merged commit e105294 into python:master Mar 26, 2018

bedevere-bot removed the awaiting merge label Mar 26, 2018

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 26, 2018

Corrected link targets in collections.rst (pythonGH-1052)

bc8ce43

(cherry picked from commit e105294) Co-authored-by: Michael Seifert <michaelseifert04@yahoo.de>

bedevere-bot removed the needs backport to 3.7 label Mar 26, 2018

miss-islington added a commit that referenced this pull request Mar 26, 2018

Corrected link targets in collections.rst (GH-1052)

d01a805

(cherry picked from commit e105294) Co-authored-by: Michael Seifert <michaelseifert04@yahoo.de>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Corrected link targets in collections.rst #1052

Corrected link targets in collections.rst #1052

MSeifert04 commented Apr 8, 2017 •

edited

Loading

mention-bot commented Apr 8, 2017

serhiy-storchaka left a comment

serhiy-storchaka Apr 8, 2017

serhiy-storchaka Apr 8, 2017

serhiy-storchaka Apr 8, 2017

MSeifert04 Apr 8, 2017

serhiy-storchaka Apr 8, 2017

serhiy-storchaka Apr 8, 2017

vadmium Apr 9, 2017

orsenthil left a comment

serhiy-storchaka commented Apr 10, 2017

Mariatta commented Apr 10, 2017

rhettinger commented Apr 11, 2017

MSeifert04 commented Apr 12, 2017

MSeifert04 commented May 12, 2017 •

edited

Loading

MSeifert04 commented Sep 7, 2017

MSeifert04 commented Sep 16, 2017

MSeifert04 commented Sep 23, 2017

serhiy-storchaka commented Sep 23, 2017

MSeifert04 commented Sep 23, 2017 •

edited

Loading

serhiy-storchaka commented Sep 23, 2017

MSeifert04 commented Sep 25, 2017 •

edited

Loading

MSeifert04 commented Oct 7, 2017

miss-islington commented Mar 26, 2018

bedevere-bot commented Mar 26, 2018

miss-islington commented Mar 26, 2018

		@@ -1175,6 +1176,11 @@ attribute.
		subclass) or an arbitrary sequence which can be converted into a string using

Corrected link targets in collections.rst #1052

Corrected link targets in collections.rst #1052

Conversation

MSeifert04 commented Apr 8, 2017 • edited Loading

mention-bot commented Apr 8, 2017

serhiy-storchaka left a comment

Choose a reason for hiding this comment

serhiy-storchaka Apr 8, 2017

Choose a reason for hiding this comment

serhiy-storchaka Apr 8, 2017

Choose a reason for hiding this comment

serhiy-storchaka Apr 8, 2017

Choose a reason for hiding this comment

MSeifert04 Apr 8, 2017

Choose a reason for hiding this comment

serhiy-storchaka Apr 8, 2017

Choose a reason for hiding this comment

serhiy-storchaka Apr 8, 2017

Choose a reason for hiding this comment

vadmium Apr 9, 2017

Choose a reason for hiding this comment

orsenthil left a comment

Choose a reason for hiding this comment

serhiy-storchaka commented Apr 10, 2017

Mariatta commented Apr 10, 2017

rhettinger commented Apr 11, 2017

MSeifert04 commented Apr 12, 2017

MSeifert04 commented May 12, 2017 • edited Loading

MSeifert04 commented Sep 7, 2017

MSeifert04 commented Sep 16, 2017

MSeifert04 commented Sep 23, 2017

serhiy-storchaka commented Sep 23, 2017

MSeifert04 commented Sep 23, 2017 • edited Loading

serhiy-storchaka commented Sep 23, 2017

MSeifert04 commented Sep 25, 2017 • edited Loading

MSeifert04 commented Oct 7, 2017

miss-islington commented Mar 26, 2018

bedevere-bot commented Mar 26, 2018

miss-islington commented Mar 26, 2018

MSeifert04 commented Apr 8, 2017 •

edited

Loading

MSeifert04 commented May 12, 2017 •

edited

Loading

MSeifert04 commented Sep 23, 2017 •

edited

Loading

MSeifert04 commented Sep 25, 2017 •

edited

Loading