8000 bpo-29428: make doctest documentation clearer by JDLH · Pull Request #45 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

bpo-29428: make doctest documentation clearer #45

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

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address bitdancer review comments, Doc/library/doctest.rst
  • Loading branch information
Jim DeLaHunt committed Feb 13, 2017
commit a140a7fc144f7e03ff94127731633e92320d56cd
65 changes: 33 additions & 32 deletions Doc/library/doctest.rst
Original file line number Diff line number Diff line change
Expand Up @@ -301,20 +301,19 @@ 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.
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.
A doctest example is composed of one or more tests. An individual test
Copy link
Member

Choose a reason for hiding this comment

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

This looks wrong, an example (that you name test) starts with >>> and ends after its output.

Meaning this is a correct example not covered by your paragraph:

>>> say = "Ni"
>>> print(say)
Ni

This is already covered by:

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.

Please remove this paragraph.

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.

Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
but doctest isn't trying to do an exact emulation of the Python shell.
Copy link
Member

Choose a reason for hiding this comment

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

There exist more than one Python shell, please leave "any specific".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reviewers are not consistent. Back on the first commit, I kept the wording "any specific Python shell". @bitdancer 's comment on Feb 12, 2017 was:

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.
My reply was:
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".

What wording will satisfy both @bitdancer and @JulienPalard ?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the inconsistencies, both are true, I still prefer "any specific", but "the" is simplier. In any cases the message passes about "don't rely on copy/paste to work 100% of the times".

Again, this modification is not linked to your original issue, when we're saying "try to propose atomic changes" we're having this in mind: The more you're modifying, the more people will discuss it, this is also why I originally said "please leave" this as is.


Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
...
>>> import math
>>> x = factorial(10); math.ceil(math.log10(x))
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using an import statement in the example is at the heart of why I'm offering this revision. I'm glad it works. I agree that having a statement list is unusual. However, 1) it's an unobtrusive way of demonstrating that the doctest module looks for statement lists, not statements, and 2) I actually use statement lists in some of my doctests. But this is a minor point, I'm willing to bow to wisdom of the more experienced contributors.

7
Expand All @@ -329,24 +328,27 @@ but doctest isn't trying to do an exact emulation of any specific Python shell.

The fine print:

* The ``>>>`` string tells the module to look for an interactive statement:
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
statement. (Actually, doctest gets these strings from the PS1 and PS2
values of the :mod:`sys` module.)
* The >>> marks the start of an interactive statement: that is, a
statement list ending with a newline, or a :ref:`compound statement <compound>`.
The ... string indicates that the line continues a compound statement.

* 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
* If the expected output is empty, it indicates that the test generates
Copy link
Member

Choose a reason for hiding this comment

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

The documentation use the word example, not test, please use it here too:

If the expected output is empty, it indicates that the example generates no output when run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

8000

I believe that Doc/library/doctest.rst is inconsistent in how it uses the words "example", "test", and "docstring". I was trying to make the wording consistent. But this isn't the heart of what I want to do with this P.R. Will you accept it if I make each paragraph keep using the same word "test" or "example" that it currently uses?

no output when run. If the test does generate output, the module reports
it as a failure.
Copy link
Member

Choose a reason for hiding this comment

The 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..."

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
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.
Copy link
Member

Choose a reason for hiding this comment

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

*example


* 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.

* 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
blank line, put ``<BLANKLINE>`` in your doctest test each place a blank line
Copy link
Member

Choose a reason for hiding this comment

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

I disagree, the doctest module is not aimed to write tests in docstrings, but to test examples found in docstrings. So here example was the right word.

is expected.

* All hard tab characters are expanded to spaces, using 8-column tab stops.
Expand Down Expand Up @@ -390,29 +392,28 @@ The fine print:
1

and as many leading whitespace characters are stripped from the expected output
as appeared in the initial ``'>>> '`` line that started the example.
as appeared in the initial ``'>>> '`` line that started the test.
Copy link
Member

Choose a reason for hiding this comment

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

same.



.. _doctest-execution-context:

What's the Execution Context?
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Within a docstring, later examples can use names defined by earlier
examples. It's fine for an example to set up state, and
Within a docstring, later tests can use names defined by earlier
Copy link
Member

Choose a reason for hiding this comment

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

I would not say so, exemples should be as autonomous as possible to make sense, one should not write an example containing only setup statements, then other examples using them, it's possible, but it's not a feature, just a bad practice, let's not encourage it. Still one can put setup statement in its example so the example is reproductible by itself (autonomous). Please remove this paragraph.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JulienPalard, the core of this PR is that I wanted to test some code which required an import of another module to run. Is this "bad practice"? If I think I need to import a module to make an "example" run, should I stop using doctests and put that code in a unittest fixture instead?

Copy link
Member

Choose a reason for hiding this comment

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

It's not a bad practice to use import in a doctest, I'm saying you don't need two separate doctests in the same docstring to do so, a single doctest is enough, like:

"""
An example:
>>> import math
>>> math.sqrt(4)
2.0
"""

I do not even consider bad practice separating imports and examples, like:

"""
An example:
First do the import:
>>> import math

Then use sqrt:
>>> math.sqrt(4)
2.0
"""

Yet it's harder to read, it's visually two distincts sessions, with a lot of context switch (text, repl text, repl). It would make more sense when demoing 4 different features, avoiding to import 4 times.

The point of my comment: you're stating "Within a docstring, later tests can use names defined by earlier" which is generalizing to variables, not only imports, and I see very few examples where it can be readable with shared variables. After reading this line, I image one writing:

def foo():
    """
    First example with division:
    >>> a = 10 / 3
    >>> print(a)
    3.3333333333333335

    Second example, with floor division:
    >>> a // 2
    1.0
    """

Which is just bad (this is what I consider "bad practice"): the second example will work under doctest, but one every other user will probably just be interested by the second example, which will fail if they try it alone. This is why I'm speaking of "autonomous examples".

examples. It's fine for an test to set up state, and
have no output.
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 import statement is handled in doctests. This is related to concepts of how the state accumulates across examples within a docstring, and how state is reset between docstrings. This paragraph does a poor job of describing how state is reset between docstrings. That is why I try to improve it.

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,
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.
*shallow copy* of :mod:`M`'s globals. This means tests can freely
use any names defined at the top level of :mod:`M`.
When doctest performs tests, it doesn't change the module's real globals.

This shallow copy of globals is discarded after the docstring has been
processed, and copied afresh for the next docstring. Thus, tests in one
docstring in :mod:`M` can't leave behind crumbs that accidentally allow an
test in another docstring to work. Tests cannot see names defined in
other docstrings.

You can force use of your own dict as the execution context by passing
``globs=your_dict`` to :func:`testmod` or :func:`testfile` instead.
Expand Down
0