-
-
Notifications
You must be signed in to change notification settings - Fork 31k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-29428: make doctest documentation clearer #45
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -276,8 +276,9 @@ sections. | |
Which Docstrings Are Examined? | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
The module docstring, and all function, class and method docstrings are | ||
searched. Objects imported into the module are not searched. | ||
The docstring for the module, and the docstrings for all functions, | ||
classes, and methods in that module, are searched. | ||
Objects imported into the module are not searched. | ||
|
||
In addition, if ``M.__test__`` exists and "is true", it must be a dict, and each | ||
entry maps a (string) name to a function object, class object, or string. | ||
|
@@ -300,33 +301,49 @@ their contained methods and nested classes. | |
How are Docstring Examples Recognized? | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
The :mod:`doctest` module treats any line beginning with ``>>>`` as an | ||
example to be tested. | ||
Following lines which begin with ``...`` continue the example. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In doctest, 'example' generally refers to a block of lines that have no intervening text, although really it refers to an expository unit that would be very difficult to automatically parse. You were probably thinking 'test' here, but that is also incorrect, since 'test' includes the model output lines. So I think what you want to say here is "... continues the code portion of the test", but I'm not at all sure it is worth being that precise here. I would suggest leaving all this to the fine print section. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm using 'example' because I think that's what this document chose as the term for this line-beginning-with- |
||
Subsequent non-blank lines, if any, form an expected output string. | ||
A blank (all-whitespace) line, or a line with ``>>>`` (beginning the | ||
next example), ends the expected output string. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As implied above, I prefer the original exposition. The key point right here is that a copy and paste of an interactive session works. The rest should go in the fine print. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The overarching theme in this revision is, I think the document tries to say "here's how you use doctest", and imply what doctest does. This breaks down when implication isn't clear enough, the reader has a problem, and comes looking for a solution. I think it's a good approach to say "this is what doctest does", then move on to "here's how you use doctest". I think the original wording, "In most cases a copy-and-paste... works fine", was worse: it didn't say how you use doctest, it just implied it. Hence my leading with the "this is what doctest does" statement. |
||
In most cases a copy-and-paste of an interactive console session works fine, | ||
but doctest isn't trying to do an exact emulation of any specific Python shell. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps this should say "the python shell" instead of "any specific python shell", since I think most alternate shells can't be cut and pasted as doctests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sentence is verbatim from the previous version of the document. I don't have strong feelings about this change. I'm happy to go with "the python shell". |
||
:: | ||
|
||
>>> # comments are ignored | ||
>>> x = 12 | ||
>>> x | ||
12 | ||
>>> if x == 13: | ||
... print("yes") | ||
... | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks to me like a bug in the interactive shell. Let's not include the bug in the docs, and instead file a separate issue about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The bug being, the Python 3.7 in-dev interactive shell receives a comment-only line, and prints a sys.ps2 prompt There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please submit it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I take it back. Every one of my Python versions, 2.7 from Macports, 2.7 from Apple, 3.7 from master, 3.6 from Macports, all print out a sys.ps2 prompt after a comment-only line. How does your copy of Python behave? If the sys.ps2 prompt is what every current Python shell does, then I think the example should reflect that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. http://bugs.python.org/issue29561 Interactive mode gives sys.ps2 not sys.ps1 after comment-only line |
||
>>> import math | ||
>>> x = factorial(10); math.ceil(math.log10(x)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most doctests would not use ;. It will strengthen your 'multiline' example to make these two lines. Having an import does enhance the example, I think. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using an |
||
7 | ||
>>> if x == factorial(9)*10: | ||
... print("same:\n{0}".format(x)) | ||
... else: | ||
... print("no") | ||
... print("NO") | ||
... print("NO!!!") | ||
... print("differ:\n{0}\n{1}".format(x, factorial(9)*10)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is much drier than the original example, and is more code for the reader to read and understand, but that code and the knowledge gained from understanding it is a distraction from the point of the example rather than an enhancement. So again I prefer the original here. It also seems to me, looking at this original example, that it is already demonstrating the multi-line behavior that you are saying isn't covered. x is set in one test, and then interrogated in two subsequent tests. Your revision does make that point more strongly, however, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm open to a better example. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Multi-line examples having a setup phrase is already covered in the original example as @bitdancer said, in:
In your example, the line: Please do not change the example, it was simple and covered the point you're trying to fix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @JulienPalard , thank you for your review.
And the core of what I am proposing is to use a different demonstration of multiple "examples" which build up state and then use it, as in:
(I believe there is a benefit of using a statement group with You said, "Please do not change the example, it was simple and covered the point you're trying to fix." My feedback to you is that, as a developer and a reader of this documentation, it did not cover the point. It did not tell me how to use use an |
||
... | ||
no | ||
NO | ||
NO!!! | ||
same: | ||
3628800 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By deleting this intro, you've lost the context in which the subsequent detailed explanation is given. However, this existing intro doesn't give quite enough information either. I suggest: A doctest example is composed of one or more tests. An individual test starts with a line that starts with '>>>', has zero or more code continuation lines that start with '...', and ends with zero or more expected output lines. The expected output ends at the first line that starts with '>>>' or is blank. All lines in an example block must have the same indentation level. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Funny, you describe the deleted paragraph as an "intro". I looked at it as the lagging statement of "this is what doctest does", which should have been before the example. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be clear, the latest version does take @bitdancer's intro and moves it to above the example, replacing my original intro paragraph. The Github discussion doesn't make that clear. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As in a previous review I asked you to remove the new intro paragraph with wrong statement in it, please keep this one. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm sorry, @JulienPalard , I'm having a hard time finding the review where you ask me to remove the "new intro paragraph with wrong statement in it". I don't know how to respond to this feedback. |
||
>>> | ||
|
||
Any expected output must immediately follow the final ``'>>> '`` or ``'... '`` | ||
line containing the code, and the expected output (if any) extends to the next | ||
``'>>> '`` or all-whitespace line. | ||
|
||
The fine print: | ||
|
||
* The ``>>>`` string tells the module to look for an interactive statement: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "tells the module" isn't parallel to the other descriptive text in the docs. It should be something like "The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good improvement. Thank you. |
||
that is, a statement list ending with a newline, or a | ||
:ref:`compound statement <compound>`. | ||
The ``...`` string tells the module that the line continues a compound | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good improvement. Thank you. |
||
statement. (Actually, doctest gets these strings from the PS1 and PS2 | ||
values of the :mod:`sys` module.) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not true, the >>> and ... are hardcoded in doctest.py. It would be a bug if it took them from sys, since that would mean if someone changed sys.PS1 it would break all doctests if they tried to use doctest. I suppose, however, that mentioning that these are the same as the default values for sys.PS1 and sys.PS2 is worthwhile. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are correct. I had remembered incorrectly the _EXAMPLE_RE in cpython/Lib/doctest.py:582-593. We should delete the whole sentence in parentheses. I don't think that news that it resembles the default values of sys.PS[12]. |
||
|
||
* The expected output can be absent. This indicates that the example generates | ||
no output when run. If the example does generate output, the module reports | ||
it as a failure. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If my intro suggestion above is accepted, this should read "If the expected output is empty, it indicates that the..." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good improvement. Thank you. |
||
|
||
* The expected output can contain multiple lines. These lines become a | ||
string, which is compared to the string of actual output from | ||
testing the example. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These lines become a string containing newlines. The leading indentation of the example block is stripped when building the string. The resulting string is compared to the string of actual output from running the test. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good improvement. Thank you. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another point could be added: "The last code continuation line of an example copied from the interactive shell (the line starting with "..." that is otherwise blank in the example above) may be omitted without changing the meaning of the test." There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good improvement. Thank you. |
||
|
||
* Expected output cannot contain an all-whitespace line, since such a line is | ||
taken to signal the end of expected output. If expected output does contain a | ||
blank line, put ``<BLANKLINE>`` in your doctest example each place a blank line | ||
|
@@ -381,12 +398,20 @@ The fine print: | |
What's the Execution Context? | ||
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
||
By default, each time :mod:`doctest` finds a docstring to test, it uses a | ||
*shallow copy* of :mod:`M`'s globals, so that running tests doesn't change the | ||
module's real globals, and so that one test in :mod:`M` can't leave behind | ||
crumbs that accidentally allow another test to work. This means examples can | ||
freely use any names defined at top-level in :mod:`M`, and names defined earlier | ||
in the docstring being run. Examples cannot see names defined in other | ||
Within a docstring, later examples can use names defined by earlier | ||
examples. It's fine for an example to set up state, and | ||
have no output. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO this paragraph is unneeded. It is already clearly demonstrated in your enhanced example. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This new paragraph and the two following new paragraphs are my attempt to say more clearly what the old text says in one, jumbled paragraph. I like explicitly saying that state is shared within a docstring, because it's a contrast to what the next paragraph says about state between docstrings. Also, the old text buried the news about shared state within a docstring, and I'm working out my frustrations by now saying it clearly. I think this is an important point, and it's worth saying it explicitly and also showing it in the examples. |
||
|
||
For each docstring, :mod:`doctest` makes (by default) a | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're reformulating this paragraph but it is not linked to the original issue you're trying to fix. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @JulienPalard , thank you for your review. Maybe this is a philosophical issue about documentation. I believe documentation works as a related structure: a particular idea may be introduced early in the document, illustrated with examples in the middle of the document, and then defined with detailed text later in the document. So, improving how a document describes one concept may touch multiple places in a file. I worry that all the insistence on limiting changes just a few adjacent lines makes it hard to refactor the concepts in a document. It also makes it harder to review how a set of changes to the same document will affect the coherence of that document. Reformulating this paragraph is related to the original issue I'm trying to fix. The original issue is how an But if it's not possible to refactor documents, only to propose changes to one paragraph or another, then I can break this P.R. up into multiple, each trying to fix one weakness. I think that will be harder to review, and more likely to result in partial changes that are incoherent. But I will try to improve what I can improve. |
||
*shallow copy* of :mod:`M`'s globals. | ||
This means examples can freely use any names defined at the top level of | ||
:mod:`M`. | ||
When doctest tests examples, it doesn't change the module's real globals. | ||
|
||
This shallow copy of globals is discarded at the end of each docstring, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/at the end of each docstring/after the docstring has been processed/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good improvement. Thank you. |
||
and copied afresh for the next docstring. Thus, examples in one docstring | ||
in :mod:`M` can't leave behind crumbs that accidentally allow an example | ||
in another docstring to work. Examples cannot see names defined in other | ||
docstrings. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is definitely an improvement over the original text. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you. This is another instance of me working out my frustrations against the old text. |
||
|
||
You can force use of your own dict as the execution context by passing | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer the existing wording here. Your formulation uses more words without adding any clarity. This isn't a strong preference, however, so if other people prefer it I have no object. However, 'for the' should be 'of', since the docstring belongs to the object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, thank you, David, for your careful review. You have found many ways to improve this PR.
The phrase "The module docstring" caused me to stumble as I read it. The first meaning that came to mind was "The module known as docstring", that is, a module. Of course the intended meaning is "The docstring of the module", that is, a docstring. I think the original wording affords two interpretations, and the replacement affords only one. But, I don't feel strongly about this change. I just improved it on my way to the next section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for the @JDLH wording.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for disambiguating, can probably be made short again using something like: