
This issue trackerhas been migrated toGitHub, and is currentlyread-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.
Created on2008-06-21 02:31 byfer_perez, last changed2022-04-11 14:56 byadmin. This issue is nowclosed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| issue3158.diff | zach.ware,2013-07-17 04:33 | review | ||
| issue3158.v2.diff | zach.ware,2013-11-11 22:32 | Version 2 | review | |
| issue3158.__objclass__-fix.diff | zach.ware,2014-01-28 15:36 | review | ||
| Messages (20) | |||
|---|---|---|---|
| msg68489 -(view) | Author: Fernando Pérez (fer_perez) | Date: 2008-06-21 02:31 | |
Doctest fails to find doctests defined in extension modules. With toolslike cython (http://cython.org) it's trivially easy to add docstrings toextension code, a task that is much less pleasant with hand-writtenextensions. The following patch is a minimal fix for the problem:--- doctest_ori.py 2008-06-20 19:22:56.000000000 -0700+++ doctest.py 2008-06-20 19:23:53.000000000 -0700@@ -887,7 +887,8 @@ for valname, val in obj.__dict__.items(): valname = '%s.%s' % (name, valname) # Recurse to functions & classes.- if ((inspect.isfunction(val) or inspect.isclass(val)) and+ if ((inspect.isfunction(val) or inspect.isclass(val) or+ inspect.isbuiltin(val) ) and self._from_module(module, val)): self._find(tests, val, valname, module, source_lines, globs, seen)However, it is likely not sufficient as it doesn't take into account the__test__ dict, for which probably the same change would work, just a fewlines later. Furthermore, the real issue is in my view in thedistinction made by inspect between isfunction() and isbuiltin() for thesake of analyzing docstrings. isfunction() returns false for a functionthat is defined in an extension module (though it *is* a function) whileisbuiltin returns True (though it is *not* a builtin).For purposes of doctesting, doctest should simply care:- That it is a function.- That it has a docstring that can be parsed.But in too many places in doctest there are currently assumptions aboutbeing able to extract full source, line numbers, etc. Hopefully thisquick fix can be applied as it will immediately make doctest work withlarge swaths of extension code, while a proper rethinking of doctest ismade. BTW, in that process doctest will hopefully be made more modular andflexible: its current structure forces massive copy/paste subclassingfor any kind of alternate use, since it has internally hardwired use ofits own classes. Doctest is tremendously useful, but it really coulduse with some structural reorganization to make it more flexible (cleanly). | |||
| msg68491 -(view) | Author: Amaury Forgeot d'Arc (amaury.forgeotdarc)*![]() | Date: 2008-06-21 07:52 | |
For me, a 'function' is written in Python, a 'builtin' is written in C. The fact that it is defined in an extension module is irrelevant, anddepends on the implementation: zlib is an extension module on Unix, butis linked inside python26.dll on Windows.maybe inspect.isroutine() is the correct test here. | |||
| msg68525 -(view) | Author: Fernando Pérez (fer_perez) | Date: 2008-06-21 17:50 | |
I think there are two issues that need to be separated:1. The doctest bug. I'm happy with any resolution for it, and I'm notclaiming that my patch is the best approach. isroutine() indeed worksin my case, and if that approach works well in general for doctest, I'mperfectly happy with it.2. Terminology. I really disagree with the idea that - 'function' describes the implementation language of an object insteadof whether it's a standalone callable (vs an object method).- 'builtin' doesn't mean the object is "built into the shipped Python"but instead that it's "written in C". The language exposes its builtins via the __builtin__ module preciselyto declare what is part of itself, and it even has in the documentation:http://docs.python.org/lib/built-in-funcs.htmla section that starts:"""2.1 Built-in FunctionsThe Python interpreter has a number of functions built into it that arealways available."""Nowhere does it say that "builtins are written in C and functions inPython".In summary, I'm happy with any fix for the bug, but I very stronglydisagree with a use of terminology that is confusing and misleading (andwhich unfortunately is enshrined in the inspect and types modules in howthey distinguish 'Function' from 'BuiltinFunctionType').And by the way, by 'extension module' I mean to describe C-extensions,since that is how most C code is shipped by third-party authors, thoseaffected by this bug (since the stdlib doesn't seem to use doctestsitself for its own testing of C code). | |||
| msg91527 -(view) | Author: Alexey Shamrin (ash) | Date: 2009-08-13 18:04 | |
I've added Tim Peters to the nosy list. Is there anyone else who shouldbe considered as doctest maintainer? I've also checked further Pythonversions - a quick a look at latest doctest source shows that theproblem is still there.There are some details (and a workaround) in Cython FAQ:http://wiki.cython.org/FAQ#HowcanIrundoctestsinCythoncode.28pyxfiles.29.3F | |||
| msg193208 -(view) | Author: Zachary Ware (zach.ware)*![]() | Date: 2013-07-17 04:33 | |
In looking at test_decimal forissue16748, I discovered that not all of the doctests for _decimal are being tested. So, I fixed it (or at least tried to :).This patch does the following:- Replace most inspect.isfunction checks in DocTestFinder with inspect.isroutine- Add a check to DocTestFinder._from_module for method_descriptors- Correct a doctest on float.fromhex (which is incorrect back to 2.7) due to the new float repr- Add a couple doctests to the builtins module because previously there were no functions in builtins with docstrings for testing against. I chose to add to bin(), hex(), and oct(); I believe those three can benefit from having the docstring show the format of the output string. I'm not terribly attached to those, though, so if anyone has a better suggestion for testing, I'm all ears.- Add tests using the builtins module (since it's a C module that's definitely going to be available).- Add doctests to test_builtin (because the tests aren't actually run in test_doctest, just found). While at it, convert test_builtin to unittest.main(). I believe test_builtin should grow doctest running even if the doctests I added to bin, hex, and oct aren't kept; float and int have some doctests that should be kept up to date.- Add notes to the docs. | |||
| msg195709 -(view) | Author: Zachary Ware (zach.ware)*![]() | Date: 2013-08-20 18:33 | |
Ping!Anyone able to do a review of this patch? It still applies cleanly to default (or even 3.3, if this qualifies as a bug rather than a new feature). | |||
| msg202402 -(view) | Author: R. David Murray (r.david.murray)*![]() | Date: 2013-11-08 02:58 | |
Added some review comments.Because it could cause possibly buggy doctest fragments to run that previously did not run, I don't think it should be backported as a bug fix. | |||
| msg202654 -(view) | Author: Zachary Ware (zach.ware)*![]() | Date: 2013-11-11 22:32 | |
Here's a new version of the patch that I think addresses the points in your review (which I've also replied to on Rietveld).And I agree about not backporting. | |||
| msg203431 -(view) | Author: Zachary Ware (zach.ware)*![]() | Date: 2013-11-19 21:52 | |
Does this qualify as a new feature that needs to be in before beta 1? | |||
| msg203463 -(view) | Author: Eric Snow (eric.snow)*![]() | Date: 2013-11-20 07:34 | |
Larry: thoughts? | |||
| msg204174 -(view) | Author: Zachary Ware (zach.ware)*![]() | Date: 2013-11-24 03:52 | |
In the absence of input, I'm going to go ahead and commit this just in case in needs to be in before feature freeze, but I'll leave the issue open at "commit review" stage for a few days in case anyone does have something to say about it. | |||
| msg204185 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2013-11-24 07:20 | |
New changeset95dc3054959b by Zachary Ware in branch 'default':Issue#3158: doctest can now find doctests in functions and methodshttp://hg.python.org/cpython/rev/95dc3054959b | |||
| msg204186 -(view) | Author: Zachary Ware (zach.ware)*![]() | Date: 2013-11-24 07:26 | |
One-year-olds don't like productivity. Committed, 3 hours after I said I would :). I'll leave this open for a couple days just in case. | |||
| msg204190 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2013-11-24 08:22 | |
New changeset30b95368d253 by Zachary Ware in branch 'default':Issue#3158: Relax new doctests a bit.http://hg.python.org/cpython/rev/30b95368d253 | |||
| msg209495 -(view) | Author: Thomas Kluyver (takluyver)* | Date: 2014-01-28 01:31 | |
I think there's an issue with this change - ismethoddescriptor() doesn't guarantee that that the object has an __objclass__ attribute. Unbound PyQt4 signals appear to be a case where this goes wrong.This came up testing IPython on Python 3.4 - we subclass DocTestFinder, which creates other problems, but it looks like it would run into trouble even with the base implementation.https://github.com/ipython/ipython/issues/4892 | |||
| msg209555 -(view) | Author: Zachary Ware (zach.ware)*![]() | Date: 2014-01-28 15:36 | |
Does this patch fix things for you? | |||
| msg209592 -(view) | Author: Julian Taylor (jtaylor) | Date: 2014-01-28 21:49 | |
the patch seems to work for me in ipython. | |||
| msg210422 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2014-02-06 21:47 | |
New changesetc964b6b83720 by Zachary Ware in branch 'default':Issue#3158: Provide a couple of fallbacks for in case a method_descriptorhttp://hg.python.org/cpython/rev/c964b6b83720 | |||
| msg210424 -(view) | Author: Zachary Ware (zach.ware)*![]() | Date: 2014-02-06 21:57 | |
Done. | |||
| msg213166 -(view) | Author: Roundup Robot (python-dev)![]() | Date: 2014-03-11 19:13 | |
New changeset8520e0ff8e36 by R David Murray in branch 'default':whatsnew: doctest finds tests in extension modules (#3158)http://hg.python.org/cpython/rev/8520e0ff8e36 | |||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:56:35 | admin | set | github: 47408 |
| 2014-03-11 19:13:33 | python-dev | set | messages: +msg213166 |
| 2014-02-06 21:57:41 | zach.ware | set | status: open -> closed resolution: fixed messages: +msg210424 stage: commit review -> resolved |
| 2014-02-06 21:47:08 | python-dev | set | messages: +msg210422 |
| 2014-01-28 21:49:09 | jtaylor | set | nosy: +jtaylor messages: +msg209592 |
| 2014-01-28 15:36:55 | zach.ware | set | status: closed -> open files: +issue3158.__objclass__-fix.diff resolution: fixed -> (no value) messages: +msg209555 |
| 2014-01-28 01:31:36 | takluyver | set | nosy: +takluyver messages: +msg209495 |
| 2013-11-27 16:11:01 | zach.ware | set | status: open -> closed |
| 2013-11-24 08:22:23 | python-dev | set | status: pending -> open messages: +msg204190 |
| 2013-11-24 07:26:28 | zach.ware | set | status: open -> pending messages: +msg204186 assignee:zach.ware resolution: fixed stage: commit review |
| 2013-11-24 07:20:30 | python-dev | set | nosy: +python-dev messages: +msg204185 |
| 2013-11-24 03:52:35 | zach.ware | set | messages: +msg204174 |
| 2013-11-20 07:34:12 | eric.snow | set | nosy: +larry messages: +msg203463 |
| 2013-11-19 21:52:38 | zach.ware | set | messages: +msg203431 |
| 2013-11-11 22:32:20 | zach.ware | set | files: +issue3158.v2.diff messages: +msg202654 |
| 2013-11-08 02:58:43 | r.david.murray | set | nosy: +r.david.murray messages: +msg202402 |
| 2013-08-20 18:42:13 | eric.snow | set | nosy: +eric.snow |
| 2013-08-20 18:33:08 | zach.ware | set | messages: +msg195709 |
| 2013-07-17 04:33:33 | zach.ware | set | files: +issue3158.diff type: enhancement versions: + Python 3.4, - Python 3.1, Python 2.7, Python 3.2 keywords: +patch nosy: +zach.ware messages: +msg193208 |
| 2010-08-03 21:36:19 | terry.reedy | set | versions: - Python 2.6, Python 2.5 |
| 2009-08-13 18:04:31 | ash | set | nosy: +tim.peters,ash messages: +msg91527 versions: + Python 2.6, Python 3.1, Python 2.7, Python 3.2 |
| 2008-06-21 17:50:13 | fer_perez | set | messages: +msg68525 |
| 2008-06-21 07:52:19 | amaury.forgeotdarc | set | nosy: +amaury.forgeotdarc messages: +msg68491 |
| 2008-06-21 02:31:44 | fer_perez | create | |