msg267166 - (view) |
Author: Jelle Zijlstra (JelleZijlstra) * |
Date: 2016-06-03 21:13 |
I'm going to gather together a patch to fix some of these issues.
|
msg267209 - (view) |
Author: Jelle Zijlstra (JelleZijlstra) * |
Date: 2016-06-04 00:48 |
Added a number of more specific issues with patches. I'll add more patches after the current ones are resolved.
|
msg267275 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2016-06-04 17:03 |
Thanks for all of these patches, but I'm -1 on committing them. We already have an extensive test suite and I'm pretty sure that all of these examples are already tested in Lib/test [*]. Learning reST is already hard for contributors and we shouldn't make it harder by introducing new directives like 'testsetup'.
* We have also converted some doctests to proper unittests in Lib/test in the past.
|
msg267278 - (view) |
Author: Jelle Zijlstra (JelleZijlstra) * |
Date: 2016-06-04 17:11 |
I would consider this a test of the quality of the documentation, not of the implementation. It's important that the examples in the documentation are actually correct. For example, a doctest in Doc/library/datetime.rst passes an "hours=" keyword argument to datetime.time when "hour=" is correct. It's going to be hard to find such bugs in examples without an automated check like `make doctest`.
I'm open to simplifying the patches by removing testsetup blocks. For what it's worth though, the docs already contain 17 testsetup blocks.
|
msg267284 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2016-06-04 17:25 |
Yes, the point here is to test the docs, not the code, and is definitely worthwhile. If we can get make doctest to pass, then the docs buildbot can start running it.
|
msg267373 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2016-06-05 04:21 |
I have also found some markup errors via 'make doctest' so I agree that this can be helpful :)
|
msg272315 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-08-10 06:06 |
New changeset f3a04d19e5cb by Zachary Ware in branch '3.5':
Issue #27200: Fix doctests in Doc/library/hashlib.rst
https://hg.python.org/cpython/rev/f3a04d19e5cb
New changeset d0302d8ecbc1 by Zachary Ware in branch 'default':
Issue #27200: Merge with 3.5
https://hg.python.org/cpython/rev/d0302d8ecbc1
|
msg272316 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2016-08-10 06:19 |
All of the current dependencies have been committed. I still see failures in:
Doc/faq/programming.rst
Doc/library/configparser.rst (also leaves Doc/example.ini)
Doc/library/copyreg.rst
Doc/library/ctypes.rst
Doc/library/datetime.rst
Doc/library/functions.rst
Doc/library/ipaddress.rst
Doc/library/pathlib.rst
Doc/library/re.rst
Doc/library/reprlib.rst
Doc/library/shlex.rst
Doc/library/turtle.rst (possibly only due to lack of usable tkinter)
Doc/library/unittest.mock-examples.rst
Doc/library/unittest.mock.rst
Doc/library/urllib.parse.rst
Doc/library/weakref.rst
For a total of 450 failures (1 in setup code) out of 1726 tests on 3.6. Also, some test leaves 'Doc/foo'.
|
msg272317 - (view) |
Author: Jelle Zijlstra (JelleZijlstra) * |
Date: 2016-08-10 06:31 |
Thanks for committing all of these patches! I have local patches for some more tests; I'll try to find some time to upload them.
|
msg272355 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2016-08-10 18:06 |
Thanks for creating all the patches! If you create further issues for this, feel free to make me nosy.
|
msg282309 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2016-12-03 23:15 |
#28860 has a patch for configparser. Jelle, if you have more patches for some of the items listed above, please upload them so Marco can focus on the others.
|
msg282310 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2016-12-03 23:19 |
#10225, still open, has a old 3.2 patch that might be useful. If it is not, it should be closed. Note that Raymond does not like the patch for the sorting help doc.
|
msg282330 - (view) |
Author: Marco Buttu (marco.buttu) * |
Date: 2016-12-04 10:53 |
Here is a patch that makes all Doc/library/datetime.rst doctests pass. The tests only pass with Python 3.6 and 3.7 (the optional ``timespec`` argument of ``datetime.isoformat()`` has been introduced in Python 3.6).
To apply the patch you should agree with issue 28863. In particular, to make the patch effective, as explained in issue 28863, you should:
* apply the Doc/conf.py patch of issue 28863.
* rename Doc/includes/tzinfo-examples.py to Doc/includes/tzinfo_examples.py
About the Raymond disappointment expressed in #10225, I see two ways to make both Raymond happy and the code examples tested. For the examples included from Doc/includes, using the ``literalinclude`` Sphinx directive, we can just add the Doc/includes path in Doc/conf.py (see issue 28863). At this point we can import them in the shell code examples (or in the ``setupcode``). In the datetime.rst.patch in attachment, I make use of this solution. Another option, for short snippets of code, not included from Doc/includes, is to use the ``testcode`` Sphinx directive.
|
msg282651 - (view) |
Author: Marco Buttu (marco.buttu) * |
Date: 2016-12-07 18:45 |
This last patch (make_doctest_ok.patch) makes all doctests pass (Sphinx 1.5, Python 3.6). Before applaying the patch there are 466 failures in 2096 tests, and after we have more tests (2414) and 0 failures. Actually, sometimes 3 tests fail in Doc/library/unittest.mock-examples.rst, but I did not get why :/
|
msg282788 - (view) |
Author: Marco Buttu (marco.buttu) * |
Date: 2016-12-09 16:23 |
I isolated all snippets in the unittest.mock documentation, and now all doctests pass without surprises :-)
|
msg284499 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2017-01-02 20:42 |
I left a few comments on rietveld,
In general, if making the doctest pass entails adding superfluous code (e.g. sorted(), <BLANKLINES>, #doctest: +SKIP (if visible), setups/teardowns, or even whole files) or removing details that might be useful (ids or filenames in reprs), I'd rather skip the snippet altogether.
Keep in mind that most of those snippets are there to document, not for testing, so imho the priority should be keeping them clear and to the point.
The behavior of those snippets should already be covered by unittest, so the only downside of skipping is that, if the behavior changes, the examples will be wrong until someone notices.
If others agree, this should make the patch simpler and easier to review; if not, breaking it down in 3-4 patches that address separate issue might also help reviewers.
|
msg284520 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2017-01-03 01:00 |
I agree with Ezio.
|
msg284548 - (view) |
Author: Marco Buttu (marco.buttu) * |
Date: 2017-01-03 11:01 |
Hi Ezio, thanks for your time :) The +SKIP option and the testsetup and teardown directives are not visible in the output. The only extra code in the output is sorted(). I agree with you about the use of sorted(), and I think we do not have to add extra code just to make the example passes. But as you can see in issue28860, I was asked to add it. Before going on, I think we should have a clear policy about it.
I also think the examples are there do document, but I believe there are good reasons to test them. In fact, unittests and doctests have complete diffent purposes: the goal of doctests is to verify the examples work properly, that's all. An example could be wrong also if all the related unittests pass and it is still updated: there could be a typo in the code or in the output, a missing import, and other kind of errors or lacks that make the example not working, getting confused the reader. Unluckily there is not just the downside you pointed out :/
Ok to break the patch in more patches, but I did not get if you want me to open a new issue for each patch, or attach all the patches here in this issue. Thanks again
|
msg284560 - (view) |
Author: Ezio Melotti (ezio.melotti) * |
Date: 2017-01-03 13:22 |
> The +SKIP option and the testsetup and teardown directives are not visible in the output.
So feel free to use them when needed :)
> But as you can see in issue28860, I was asked to add it. Before going
> on, I think we should have a clear policy about it.
If the priority is making the test pass, then adding sorted() is a viable solution -- if keeping the examples simple and to the point has higher priority, then the test could be skipped. The priority depends on the case.
> An example could be wrong also if all the related unittests pass and
> it is still updated: there could be a typo in the code or in the
> output, a missing import, and other kind of errors or lacks that make > the example not working, getting confused the reader.
True. In general, I think most of the current examples are fine, and whoever adds new ones or changes the behavior should take care that the examples (still) work. If there is a mistake in an untested example, hopefully someone will report it. While it's true that these mistakes might cause confusion, the same could be said for other changes done to the code to make the test pass (Why do I need to call sorted()/list()/etc.? Why there are '...' in the output, is this an actual terminal session? etc.).
> Ok to break the patch in more patches, but I did not get if you want
> me to open a new issue for each patch, or attach all the patches here
> in this issue. Thanks again
It's probably fine to keep the discussion and patches on this issue, unless you prefer to handle discussions about specific parts separately -- just make sure that it's clear from the file description what patches should be applied.
As for the number of patches, probably 2-4 files are enough (e.g. one file could include all the trivial addition of +SKIP/.. doctest::, another the more "controversial" changes that add new files).
|
msg284628 - (view) |
Author: Marco Buttu (marco.buttu) * |
Date: 2017-01-04 11:06 |
Thinking a little bit about it, I believe that the easiest way to proceed is to make one patch for every file or two, with the only purpose to make their doctests to pass. Otherwise if I make a patch with all the doctest directives, or just with the +SKIP options, we will have hundreds of failures, and it will be really hard to understand what is going wrong, also because the tests are not isolated, and we will have some random failures. Reducing the number of failures by making a patch with all the doctest/testsetup/testcleanup directives is not a solution, because we eventually fall again into a big patch, and the doctests are still not pass too.
That is why I think having one patch for two files will reduce a lot the effort in making and reviewing the patch. We will end up in about 9 patches, and it will be really easy to review most of them (I think couple of minutes). It will be also easy to test them. For instance, if the patch involves the files Doc/library/copyreg.rst and Doc/library/ctypes.rst, we just have to to to the Doc/ directory and run:
$ sphinx-build -b doctest . build/doctest \
> library/copyreg.rst \
> library/ctypes.rst
expecting 0 failures.
Maybe having one new issue for each patch will also simplify the process, because the purpose of the patch will be well explained by the issue description and title. Let me know what you think, and thank you very much for your time.
|
msg286413 - (view) |
Author: Marco Buttu (marco.buttu) * |
Date: 2017-01-28 16:06 |
In order for the patches to work for every Python version (see #msg284622 ), I added a new feature to the Sphinx doctest extension, called pyversion. Starting from Sphinx 1.6, it will be possible to specify the Python version in order for the example to be tested. For instance, if I run the doctests with Python 3.3, then this test will be skipped:
.. doctest::
:pyversion: > 3.4
# Code example
It will be tested only when Sphinx is running on Python > 3.4. My question is: what about using this feature in the patches? I am asking this because Sphinx 1.6 will be released in about 7-10 months, so until then, if we want to use the :pyversion: feature we have to build the documentation using the master version of Sphinx. Please, let me know
|
msg286463 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2017-01-30 00:08 |
I'm -1 on building the docs from Sphinx master as I don't want to complicate our development process to deal with potential broken tooling. Basically if the doctests don't pass on a compiled checkout for the version of Python the docs are being built for then the doctest is broken (i.e. no need to care about compatibility with older Python versions for docs meant for master).
|
msg286490 - (view) |
Author: Marco Buttu (marco.buttu) * |
Date: 2017-01-30 13:19 |
Thanks Brett :-) Following the suggestion of Ezio to split the patch in more patches (msg284560), and according to msg284628, here is a first patch for just three files. To run their doctests:
$ sphinx-build -b doctest . build/doctest \
library/configparser.rst \
library/ctypes.rst \
library/copyreg.rst
|
msg286510 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2017-01-31 03:36 |
Mariatta, would you please add your review comments to the proposed patches. For the most part, we want the patches to make the minimal changes necessary to pass the doctests. Also, clarity of the examples is more important than making the examples doctestable (i.e. perhaps added sorted() to the output makes it more predictable but it might also obfuscate the point being made).
In general, we would like to have the convenience of doc testing but should be unwilling to commit any atrocities along the way. Ideally, the examples should be written in a way that doesn't garbage them up with <blankline> and whatnot (that is generally distracting to the reader and only serves doctest). Similarly, make sure that clarity and cut-and-pastability don't get lost by adding ... PS2 prompts that uglify the examples. In all cases, the intent of the example writer and the clarity of the example takes precedence over the doctests.
You should try out the patches and make sure the tests run, build the docs and make sure the examples are clean looking from a user point of view. If you find any issues or have questions, use rietveld for discussion (that is the "review" link to the right of the patch).
|
msg286511 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2017-01-31 03:37 |
Also, please make this 3.7 only. There is no point in going back in time unless an example is found that is actually broken.
|
msg286514 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2017-01-31 03:45 |
Raymond Hettinger wrote:
> Similarly, make sure that clarity and cut-and-pastability don't get lost by adding ... PS2 prompts that uglify the examples.
Note that Sphinx adds a ">>>" button to the upper right of the example
that, when clicked, makes all of the prompts and output disappear so
that the code (and only the code) can be copied easily. Not having
the ps2 prompts breaks that button.
|
msg286615 - (view) |
Author: Marco Buttu (marco.buttu) * |
Date: 2017-02-01 11:26 |
Thank you Raymond for your time. I just want to note that <blankline> does not distrac the reader, because it is not show in the output. Furthermore, to doctest an example we do not need to add the prompt (>>>): we can use the testcode/testoutput Sphinx directives.
|
msg290167 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2017-03-24 22:12 |
New changeset 909a6f626ff343937cd3f06fda996870e7890724 by Berker Peksag (Marco Buttu) in branch 'master':
bpo-27200: Fix doctests in programming.rst and datetime.rst (#401)
https://github.com/python/cpython/commit/909a6f626ff343937cd3f06fda996870e7890724
|
msg290347 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2017-03-24 23:15 |
New changeset b2a7c2f9862b28cf5db11ef837646ef0c589955c by Berker Peksag (Marco Buttu) in branch 'master':
bpo-27200: fix configparser, copyreg and ctypes doctests (#240)
https://github.com/python/cpython/commit/b2a7c2f9862b28cf5db11ef837646ef0c589955c
|
msg291617 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2017-04-13 14:18 |
New changeset 7b2491a6aa5cdc1f8f9e3fd9df91f29ee69aa982 by Berker Peksag (Marco Buttu) in branch 'master':
bpo-27200: Fix pathlib, ssl, turtle and weakref doctests (GH-616)
https://github.com/python/cpython/commit/7b2491a6aa5cdc1f8f9e3fd9df91f29ee69aa982
|
msg292431 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2017-04-27 12:23 |
New changeset e65fcde85abf6617508f2d6b77020e24b8ca6f6b by Berker Peksag (Marco Buttu) in branch 'master':
bpo-27200: Fix several doctests (GH-604)
https://github.com/python/cpython/commit/e65fcde85abf6617508f2d6b77020e24b8ca6f6b
|
msg328887 - (view) |
Author: Karthikeyan Singaravelan (xtreak) * |
Date: 2018-10-30 08:43 |
Sorry to bump this but to add some more context doctest related issues were fixed in issue34962 and is now enforced in CI.
|
msg328928 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2018-10-30 15:11 |
At the moment, the dependent issues are closed and there is just one open PR on this issue.
Julian, I am nosying you here because you merged #34962, which is properly a dependency of this.
|
msg328943 - (view) |
Author: Julien Palard (mdk) * |
Date: 2018-10-30 20:00 |
Closing PR and issue as already fixed and now enforced by CI.
Thanks again Marco for those already-merged fixes, it helped a lot ending the work at the last PyCon Fr sprints.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:31 | admin | set | github: 71387 |
2018-10-30 20:00:05 | mdk | set | status: open -> closed resolution: fixed messages:
+ msg328943
stage: patch review -> resolved |
2018-10-30 15:11:44 | terry.reedy | set | nosy:
+ mdk dependencies:
+ make doctest does not pass :/ messages:
+ msg328928
|
2018-10-30 08:43:36 | xtreak | set | nosy:
+ xtreak messages:
+ msg328887
|
2017-04-27 12:23:36 | berker.peksag | set | messages:
+ msg292431 |
2017-04-13 14:18:01 | berker.peksag | set | messages:
+ msg291617 |
2017-03-26 08:18:43 | marco.buttu | set | pull_requests:
+ pull_request727 |
2017-03-24 23:15:15 | berker.peksag | set | messages:
+ msg290347 |
2017-03-24 22:12:02 | berker.peksag | set | messages:
+ msg290167 |
2017-03-11 15:10:56 | marco.buttu | set | pull_requests:
+ pull_request509 |
2017-03-10 19:48:51 | marco.buttu | set | pull_requests:
+ pull_request499 |
2017-03-02 19:41:50 | marco.buttu | set | pull_requests:
+ pull_request332 |
2017-02-22 20:54:43 | marco.buttu | set | pull_requests:
+ pull_request202 |
2017-02-01 11:26:26 | marco.buttu | set | messages:
+ msg286615 |
2017-01-31 03:45:05 | zach.ware | set | messages:
+ msg286514 |
2017-01-31 03:37:10 | rhettinger | set | priority: normal -> low
messages:
+ msg286511 versions:
- Python 2.7, Python 3.5, Python 3.6 |
2017-01-31 03:36:13 | rhettinger | set | assignee: Mariatta
messages:
+ msg286510 nosy:
+ rhettinger, Mariatta |
2017-01-30 13:19:17 | marco.buttu | set | files:
+ configparser_ctypes_copyreg.patch
messages:
+ msg286490 |
2017-01-30 00:08:24 | brett.cannon | set | messages:
+ msg286463 |
2017-01-28 16:06:53 | marco.buttu | set | messages:
+ msg286413 |
2017-01-04 11:06:08 | marco.buttu | set | messages:
+ msg284628 |
2017-01-03 13:22:46 | ezio.melotti | set | messages:
+ msg284560 |
2017-01-03 11:01:44 | marco.buttu | set | messages:
+ msg284548 |
2017-01-03 01:00:13 | berker.peksag | set | messages:
+ msg284520 |
2017-01-02 20:42:40 | ezio.melotti | set | nosy:
+ ezio.melotti messages:
+ msg284499
|
2016-12-09 16:42:29 | zach.ware | set | assignee: JelleZijlstra -> (no value) stage: patch review versions:
+ Python 3.7 |
2016-12-09 16:23:49 | marco.buttu | set | files:
+ issue27200_3rd.patch
messages:
+ msg282788 |
2016-12-07 18:45:59 | marco.buttu | set | files:
+ make_doctest_ok.patch
messages:
+ msg282651 |
2016-12-04 10:53:43 | marco.buttu | set | files:
+ datetime.rst.patch
nosy:
+ marco.buttu messages:
+ msg282330
keywords:
+ patch |
2016-12-03 23:19:33 | terry.reedy | set | messages:
+ msg282310 |
2016-12-03 23:15:24 | terry.reedy | set | nosy:
+ terry.reedy dependencies:
+ Fixed all the doctest failures in Doc/library/configparser.rst messages:
+ msg282309
|
2016-08-10 18:06:41 | zach.ware | set | messages:
+ msg272355 |
2016-08-10 06:31:37 | JelleZijlstra | set | messages:
+ msg272317 |
2016-08-10 06:19:31 | zach.ware | set | type: behavior messages:
+ msg272316 components:
+ Documentation, Tests versions:
+ Python 2.7, Python 3.5, Python 3.6 |
2016-08-10 06:06:08 | python-dev | set | nosy:
+ python-dev messages:
+ msg272315
|
2016-08-09 21:53:35 | zach.ware | set | nosy:
+ zach.ware
|
2016-06-05 04:21:16 | berker.peksag | set | messages:
+ msg267373 |
2016-06-05 01:08:21 | JelleZijlstra | set | assignee: JelleZijlstra |
2016-06-04 17:25:21 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg267284
|
2016-06-04 17:11:51 | JelleZijlstra | set | messages:
+ msg267278 |
2016-06-04 17:03:00 | berker.peksag | set | nosy:
+ berker.peksag messages:
+ msg267275
|
2016-06-04 00:48:01 | JelleZijlstra | set | dependencies:
+ make doctest fails on 2.7 release notes, Failing doctests in Doc/faq/programming.rst, Failing doctests in Doc/howto/, Failing doctests in Library/collections.rst, Failing doctests in Doc/tutorial/, Failing doctests in Doc/whatsnew/3.2.rst, Failing doctests in Library/traceback.rst, Failing doctests in Library/email.*.rst, Failing doctests due to environmental dependencies in Lib/{nntp,ftp,diff}lib.rst messages:
+ msg267209 |
2016-06-03 21:13:27 | JelleZijlstra | create | |