bpo-22702: Improves documentation for str.join method #156

CuriousLearner · 2017-02-18T11:12:12Z

Fixes issue http://bugs.python.org/issue22702

the-knights-who-say-ni · 2017-02-18T11:12:13Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow these steps to rectify the issue:

Sign the PSF contributor agreement
Wait at least one US business day and then check "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

DimitrisJim · 2017-02-18T17:08:15Z

Hi @CuriousLearner! The original text seems fine to me, why do you think this change is warranted?

CuriousLearner · 2017-02-18T17:26:04Z

Hi @DimitrisJim !

As in the issue Raymond & others mentioned that at least the usage of repeated iterable word should be removed. Moreover, in general to make the docstring less verbose, this change is suggested.

Is there something else wanted in this PR?

DimitrisJim · 2017-02-18T17:34:21Z

Yes the double use of iterable is odd here, my view is that that is the only thing needing change.

My main issue is that the sentence "separated by the string str provided in this method" seems more confusing than the original sentence.

CuriousLearner · 2017-02-19T10:12:59Z

Sure, I've done the changes as suggested.

CuriousLearner · 2017-02-20T16:51:39Z

CLA is signed and now visible in my account.

cc @ncoghlan

zhangyangyu · 2017-02-22T10:26:06Z

Doc/library/stdtypes.rst

-   :term:`iterable` *iterable*.  A :exc:`TypeError` will be raised if there are
-   any non-string values in *iterable*, including :class:`bytes` objects.  The
-   separator between elements is the string providing this method.
+   :term:`iterable`. A :exc:`TypeError` will be raised if there are


IMHO *iterable* should be left, referring the parameter.

wm75 · 2017-03-01T16:45:31Z

I don't think anything should be changed here. The double iterable is much less disturbing when formatted as html.

marco-buttu

IMO it should be just "in iterable " and not "in the :term:iterable, as it is in the rest of the file when referring to the parameter iterable. Furthermore, you should apply the same modification to the bytearray.join() description, and add a second whitespace after the dot, before "A :exc:TypeError.

ncoghlan · 2017-04-09T09:58:05Z

@CuriousLearner Sorry for the delayed review, but I'm inclined to agree with @marco-buttu here - the iterable to drop is the term reference, rather than the parameter name.

See http://bugs.python.org/issue22702#msg291366 for further discussion.

CuriousLearner · 2017-04-12T15:48:26Z

Hi @marco-buttu & @ncoghlan !

I've made the changes according to earlier review. Please review :)

marco-buttu

Just a minor change. The rest LGTM.

marco-buttu · 2017-04-13T09:13:29Z

Doc/library/stdtypes.rst

-   separator between elements is the string providing this method.
+   Return a string which is the concatenation of the strings in *iterable*.
+   A :exc:`TypeError` will be raised if there are any non-string values in
+   *iterable*, including :class:`bytes` objects. The separator between


Could you please add an extra whitespace after the period? I mean, two whitespaces between objects. and The separator.

CuriousLearner · 2017-04-13T16:29:26Z

Hi @marco-buttu ! I've updated the patch. Can you please take a look?

marco-buttu

Thank you, ok to me :-)

berkerpeksag · 2017-05-10T10:28:05Z

Doc/library/stdtypes.rst

-   that are not :term:`bytes-like objects <bytes-like object>`, including
-   :class:`str` objects.  The separator between elements is the contents
-   of the bytes or bytearray object providing this method.
+   binary data sequences in the *iterable*.  A :exc:`TypeError` will be


You dropped "the" before *iterable* in str.join() documentation, but kept it here. Is there a reason to keep it here?

@berkerpeksag I don't remember any specific reason for that. Should I include the in str.join() documentation?

I'm not a native speaker so I'll defer the decision to @ncoghlan :)

We want to drop it - "iterable" refers specifically to the method parameter rather than the general term, so it doesn't need "the" in front of it.

@ncoghlan @berkerpeksag I've fixed it.

Is there something else needed for this PR?

ncoghlan · 2017-05-28T05:06:42Z

Added the sprint label, as this PR was submitted at the PyCon Pune 2017 core development sprint.

…onGH-156) The "iterable iterable" phrasing created confusion between the term reference and the parameter name. This simplifies the phrasing to just use the parameter name without linking directly to the term definition. (cherry picked from commit 08e2f35)

…onGH-156) The "iterable iterable" phrasing created confusion between the term reference and the parameter name. This simplifies the phrasing to just use the parameter name without linking directly to the term definition.. (cherry picked from commit 08e2f35)

…H-1896) The "iterable iterable" phrasing created confusion between the term reference and the parameter name. This simplifies the phrasing to just use the parameter name without linking directly to the term definition. (cherry picked from commit 08e2f35)

…H-1897) The "iterable iterable" phrasing created confusion between the term reference and the parameter name. This simplifies the phrasing to just use the parameter name without linking directly to the term definition. (cherry picked from commit 08e2f35)

…H-1898) The "iterable iterable" phrasing created confusion between the term reference and the parameter name. This simplifies the phrasing to just use the parameter name without linking directly to the term definition.. (cherry picked from commit 08e2f35)

…arning Since merging commit c1c47c1, the Stackless test case test_shutdown.TestShutdown.test_kill_modifies_slp_cstack_chain emits the warning: "gc:0: ResourceWarning: gc: 1 uncollectable objects at shutdown; use gc.set_debug(gc.DEBUG_UNCOLLECTABLE) to list them". This commit disables GC for this test.

Use pre-existing info instead of making another API call. python/miss-islington#153

the-knights-who-say-ni added the CLA not signed label Feb 18, 2017

serhiy-storchaka added the docs Documentation in the Doc dir label Feb 18, 2017

bpo-22702: Improves documentation for str.join method

10a1cdf

CuriousLearner force-pushed the str_join_documentation branch from e419328 to 10a1cdf Compare February 18, 2017 12:01

Address review comments for str.join documentation

2ab7eb7

Mariatta removed the CLA not signed label Feb 21, 2017

the-knights-who-say-ni added the CLA signed label Feb 21, 2017

zhangyangyu reviewed Feb 22, 2017

View reviewed changes

marco-buttu suggested changes Mar 1, 2017

View reviewed changes

Addresses review comments for str.join method documentation

27b3788

marco-buttu suggested changes Apr 13, 2017

View reviewed changes

Minor fix addressing review comments to str.join method

c243c43

marco-buttu approved these changes Apr 13, 2017

View reviewed changes

berkerpeksag reviewed May 10, 2017

View reviewed changes

Minor fix to bytearray.join() documentation addressing review comments

24fc04a

ncoghlan merged commit 08e2f35 into python:master May 27, 2017

ncoghlan added needs backport to 3.6 sprint labels May 27, 2017

Mariatta added the needs backport to 2.7 label Jun 1, 2017

Mariatta added the needs backport to 3.5 label Jun 1, 2017

Mariatta removed needs backport to 2.7 labels Jun 1, 2017

jaraco pushed a commit that referenced this pull request Dec 2, 2022

Attempt to avoid rate limit (#156)

07b9737

Use pre-existing info instead of making another API call. python/miss-islington#153

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-22702: Improves documentation for str.join method #156

bpo-22702: Improves documentation for str.join method #156

CuriousLearner commented Feb 18, 2017

the-knights-who-say-ni commented Feb 18, 2017

DimitrisJim commented Feb 18, 2017

CuriousLearner commented Feb 18, 2017

DimitrisJim commented Feb 18, 2017 •

edited

Loading

CuriousLearner commented Feb 19, 2017

CuriousLearner commented Feb 20, 2017

zhangyangyu Feb 22, 2017

wm75 commented Mar 1, 2017

marco-buttu left a comment •

edited

Loading

ncoghlan commented Apr 9, 2017

CuriousLearner commented Apr 12, 2017

marco-buttu left a comment

marco-buttu Apr 13, 2017

CuriousLearner commented Apr 13, 2017

marco-buttu left a comment

berkerpeksag May 10, 2017

CuriousLearner May 10, 2017

berkerpeksag May 10, 2017

ncoghlan May 10, 2017

CuriousLearner May 10, 2017

ncoghlan commented May 28, 2017

bpo-22702: Improves documentation for str.join method #156

bpo-22702: Improves documentation for str.join method #156

Conversation

CuriousLearner commented Feb 18, 2017

the-knights-who-say-ni commented Feb 18, 2017

DimitrisJim commented Feb 18, 2017

CuriousLearner commented Feb 18, 2017

DimitrisJim commented Feb 18, 2017 • edited Loading

CuriousLearner commented Feb 19, 2017

CuriousLearner commented Feb 20, 2017

zhangyangyu Feb 22, 2017

Choose a reason for hiding this comment

wm75 commented Mar 1, 2017

marco-buttu left a comment • edited Loading

Choose a reason for hiding this comment

ncoghlan commented Apr 9, 2017

CuriousLearner commented Apr 12, 2017

marco-buttu left a comment

Choose a reason for hiding this comment

marco-buttu Apr 13, 2017

Choose a reason for hiding this comment

CuriousLearner commented Apr 13, 2017

marco-buttu left a comment

Choose a reason for hiding this comment

berkerpeksag May 10, 2017

Choose a reason for hiding this comment

CuriousLearner May 10, 2017

Choose a reason for hiding this comment

berkerpeksag May 10, 2017

Choose a reason for hiding this comment

ncoghlan May 10, 2017

Choose a reason for hiding this comment

CuriousLearner May 10, 2017

Choose a reason for hiding this comment

ncoghlan commented May 28, 2017

DimitrisJim commented Feb 18, 2017 •

edited

Loading

marco-buttu left a comment •

edited

Loading