Minor changes in Doc/faq/library. #15449

awecx · 2019-08-24T06:26:51Z

A few improvments on faq/library doc page :

l. 162 : it should be "function" not "functions" otherwise the "behaviours" only refer to "class", which, I believe, was not intended. Thanks @christopheNan for spotting this.
l. 298 : IMO concurrent.futures introduced in 3.2 should not be considered "new" in 3.9 anymore.
l. 780 : update reference to deprecated asyncore module.

aeros

Thanks for the PR @awecx!

I some some feedback regarding the changes made:

aeros · 2019-08-24T22:36:51Z

Doc/faq/library.rst

@@ -125,7 +125,7 @@ argument list.  It is called as ::

   handler(signum, frame)

-so it should be declared with two arguments::
+so it should be declared with two parameters::


Agreed, a function is called with arguments and declared with parameters.

aeros · 2019-08-24T22:38:04Z

Doc/faq/library.rst

@@ -295,7 +295,7 @@ queue as there are threads.
 How do I parcel out work among a bunch of worker threads?
 ---------------------------------------------------------

-The easiest way is to use the new :mod:`concurrent.futures` module,
+The easiest way is to use the :mod:`concurrent.futures` module,


Agreed, concurrent.futures is no longer new.

aeros · 2019-08-24T22:40:46Z

Doc/faq/library.rst

@@ -774,10 +774,10 @@ have to check what's returned on your system.
 You can use the ``connect_ex()`` method to avoid creating an exception.  It will
 just return the errno value.  To poll, you can call ``connect_ex()`` again later
 -- ``0`` or ``errno.EISCONN`` indicate that you're connected -- or you can pass this
-socket to select to check if it's writable.
+socket to ``select()`` to check if it's writable.


If we're going to reference the method itself, a link should be provided to the corresponding documentation with Sphinx roles:

Suggested change

socket to ``select()`` to check if it's writable.

socket to :meth:`select.select` to check if it's writable.

Of course, but since "connect_ex()" and "errno.EISCONN" were only highlighted in the previous sentence, I felt I would do the same for "select()". But I agree maybe we should provide a link to those as well.

Of course, but since "connect_ex()" and "errno.EISCONN" were only highlighted in the previous sentence, I felt I would do the same for "select()".

There's quite a lot of the existing documentation which simply hasn't been updated to use the Sphinx roles, since they're a fairly new addition in comparison. Generally speaking, they are only intentionally not used when the link wouldn't provide readers any additional information, such as when an object is being referred to within it's own definition.

I see. I have made a new commit where "connect" and "connect_ex" have the link, but not "errno.EISCONN", nor "EISINPROGRESS" as their meaning is already detailed in this doc.

I see. I have made a new commit where "connect" and "connect_ex" have the link, but not "errno.EISCONN", nor "EISINPROGRESS" as their meaning is already detailed in this doc.

Just for clarification: Terms on the same doc page can be linked to if they're not defined in the same section (or in a very nearby paragraph), but that's fine.

aeros · 2019-08-24T22:41:43Z

Doc/faq/library.rst


 .. note::
-   The :mod:`asyncore` module presents a framework-like approach to the problem
+   The :mod:`asyncio` module presents a framework-like approach to the problem


Asyncore is a separate module from asyncio, see https://docs.python.org/3.5/library/asyncore.html

Of course but it is now deprecated and all all its functionnality has been moved to asyncio, hence the proposed update.

Of course but it is now deprecated and all all its functionnality has been moved to asyncio, hence the proposed update.

My point was that the two are not exactly one-to-one.

I agree that this section should be updated to recommend asyncio, but the surrounding text should be adjusted a bit as well. Their purposes are similar, but the existing text is more aimed at asyncore. Asyncio is a general purpose concurrency library, whereas asyncore was more of a framework for a specific purpose.

The :mod:`asyncio` module provides a general purpose single-threaded and concurrent asynchronous library, which can be used for writing non-blocking network code.

Edit: Slightly improved section update suggestion.

OK great, I've committed your proposal.

Doc/faq/library.rst

aeros · 2019-08-24T22:50:25Z

Doc/faq/library.rst

@@ -159,7 +159,7 @@ The "global main logic" of your program may be as simple as ::

 at the bottom of the main module of your program.

-Once your program is organized as a tractable collection of functions and class
+Once your program is organized as a tractable collection of function and class


I think this section is okay as it is. I can see where you were going with this though, you might have assumed that the "and" was implying: "... collection of functions behaviors and class behaviors" (which would be grammatically incorrect). But, in this case the two are entirely separate. The "behaviors" part is only connected to "class". The two items are:

Collection of functions

Collection of class behaviors

In that case, I don't understand why the doc states that "you should write test functions that exercise the behaviours". If "behaviours" only applies to "class" and not to "function", then the doc would say that functions not embedded into classes should not be tested. Am I missing something here ?

If "behaviours" only applies to "class" and not to "function", then the doc would say that functions not embedded into classes should not be tested.

Hmm, good point. I might've been looking at this correction too much in isolation without fully considering the surrounding context. While reading over that sentence a couple of times, I also realized it's missing a comma:

Once your program is organized as a tractable collection of function and class behaviours, you should write test functions that exercise the behaviours.

Also an interesting tidbit (this is definitely subjective though, so I wouldn't suggest changing it for this PR), but we actually alternate between the American English and British English versions of "behaviors/behaviours" within the documentation:

$ git grep -E "behaviors" -- "*.rst" | wc -l 26 $ git grep -E "behaviours" -- "*.rst" | wc -l 8

Ah nice catch (never occured to me there were two versions of this word). I've added the comma.

Co-Authored-By: Kyle Stanley <aeros167@gmail.com>

…deleted.

aeros · 2019-08-27T01:14:20Z

LGTM with the latest revisions, now we just have to wait for approval from a core developer.

bedevere-bot · 2019-09-09T15:00:47Z

@matrixise: Please replace # with GH- in the commit message next time. Thanks!

matrixise · 2019-09-09T15:01:22Z

Thank you for your contribution.

* Minor changes. * Update Doc/faq/library.rst Co-Authored-By: Kyle Stanley <aeros167@gmail.com> * Apply suggestions from aeros167. * Update Doc/faq/library.rst Co-Authored-By: Kyle Stanley <aeros167@gmail.com> * Apply suggestions from aeros167 + re-add a "a" that was accidentally deleted.

Minor changes.

8438d52

the-knights-who-say-ni added the CLA signed label Aug 24, 2019

bedevere-bot added docs Documentation in the Doc dir awaiting review labels Aug 24, 2019

aeros added skip issue skip news labels Aug 24, 2019

aeros suggested changes Aug 24, 2019

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Aug 24, 2019

awecx and others added 4 commits August 25, 2019 10:54

Update Doc/faq/library.rst

bd06e6b

Co-Authored-By: Kyle Stanley <aeros167@gmail.com>

Apply suggestions from aeros167.

129f4e3

Update Doc/faq/library.rst

57d0ea3

Co-Authored-By: Kyle Stanley <aeros167@gmail.com>

Apply suggestions from aeros167 + re-add a "a" that was accidentally …

7533b0e

…deleted.

aeros approved these changes Aug 27, 2019

View reviewed changes

matrixise merged commit 88b24f9 into python:master Sep 9, 2019

bedevere-bot removed the awaiting core review label Sep 9, 2019

	socket to ``select()`` to check if it's writable.
	socket to :meth:`select.select` to check if it's writable.

Uh oh!

Minor changes in Doc/faq/library. #15449

Minor changes in Doc/faq/library. #15449

Uh oh!

Conversation

awecx commented Aug 24, 2019

Uh oh!

aeros left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeros Aug 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeros Aug 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aeros Aug 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeros Aug 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aeros commented Aug 27, 2019

Uh oh!

bedevere-bot commented Sep 9, 2019

Uh oh!

matrixise commented Sep 9, 2019

Uh oh!

Uh oh!

aeros Aug 25, 2019 •

edited

Loading

aeros Aug 25, 2019 •

edited

Loading

aeros Aug 24, 2019 •

edited

Loading

aeros Aug 25, 2019 •

edited

Loading