Skip to content
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

Doctest fails to find doctests in extension modules #47408

Closed
ferperez mannequin opened this issue Jun 21, 2008 · 20 comments
Closed

Doctest fails to find doctests in extension modules #47408

ferperez mannequin opened this issue Jun 21, 2008 · 20 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@ferperez
Copy link
Mannequin

ferperez mannequin commented Jun 21, 2008

BPO 3158
Nosy @tim-one, @amauryfa, @larryhastings, @bitdancer, @ericsnowcurrently, @takluyver, @zware
Files
  • issue3158.diff
  • issue3158.v2.diff: Version 2
  • issue3158.objclass-fix.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/zware'
    closed_at = <Date 2014-02-06.21:57:41.800>
    created_at = <Date 2008-06-21.02:31:44.422>
    labels = ['type-feature', 'library']
    title = 'Doctest fails to find doctests in extension modules'
    updated_at = <Date 2014-03-11.19:13:33.592>
    user = 'https://bugs.python.org/ferperez'

    bugs.python.org fields:

    activity = <Date 2014-03-11.19:13:33.592>
    actor = 'python-dev'
    assignee = 'zach.ware'
    closed = True
    closed_date = <Date 2014-02-06.21:57:41.800>
    closer = 'zach.ware'
    components = ['Library (Lib)']
    creation = <Date 2008-06-21.02:31:44.422>
    creator = 'fer_perez'
    dependencies = []
    files = ['30945', '32580', '33776']
    hgrepos = []
    issue_num = 3158
    keywords = ['patch']
    message_count = 20.0
    messages = ['68489', '68491', '68525', '91527', '193208', '195709', '202402', '202654', '203431', '203463', '204174', '204185', '204186', '204190', '209495', '209555', '209592', '210422', '210424', '213166']
    nosy_count = 11.0
    nosy_names = ['tim.peters', 'fer_perez', 'amaury.forgeotdarc', 'larry', 'ash', 'r.david.murray', 'python-dev', 'eric.snow', 'takluyver', 'jtaylor', 'zach.ware']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue3158'
    versions = ['Python 3.4']

    @ferperez
    Copy link
    Mannequin Author

    ferperez mannequin commented Jun 21, 2008

    Doctest fails to find doctests defined in extension modules. With tools
    like cython (http://cython.org) it's trivially easy to add docstrings to
    extension code, a task that is much less pleasant with hand-written
    extensions. 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 few
    lines later. Furthermore, the real issue is in my view in the
    distinction made by inspect between isfunction() and isbuiltin() for the
    sake of analyzing docstrings. isfunction() returns false for a function
    that is defined in an extension module (though it *is* a function) while
    isbuiltin 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 about
    being able to extract full source, line numbers, etc. Hopefully this
    quick fix can be applied as it will immediately make doctest work with
    large swaths of extension code, while a proper rethinking of doctest is
    made.

    BTW, in that process doctest will hopefully be made more modular and
    flexible: its current structure forces massive copy/paste subclassing
    for any kind of alternate use, since it has internally hardwired use of
    its own classes. Doctest is tremendously useful, but it really could
    use with some structural reorganization to make it more flexible (cleanly).

    @ferperez ferperez mannequin added the stdlib Python modules in the Lib dir label Jun 21, 2008
    @amauryfa
    Copy link
    Member

    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, and
    depends on the implementation: zlib is an extension module on Unix, but
    is linked inside python26.dll on Windows.

    maybe inspect.isroutine() is the correct test here.

    @ferperez
    Copy link
    Mannequin Author

    ferperez mannequin commented Jun 21, 2008

    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 not
      claiming that my patch is the best approach. isroutine() indeed works
      in my case, and if that approach works well in general for doctest, I'm
      perfectly happy with it.

    2. Terminology. I really disagree with the idea that

    • 'function' describes the implementation language of an object instead
      of 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 precisely
    to declare what is part of itself, and it even has in the documentation:

    http://docs.python.org/lib/built-in-funcs.html

    a section that starts:

    """2.1 Built-in Functions

    The Python interpreter has a number of functions built into it that are
    always available."""

    Nowhere does it say that "builtins are written in C and functions in
    Python".

    In summary, I'm happy with any fix for the bug, but I very strongly
    disagree with a use of terminology that is confusing and misleading (and
    which unfortunately is enshrined in the inspect and types modules in how
    they 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, those
    affected by this bug (since the stdlib doesn't seem to use doctests
    itself for its own testing of C code).

    @ash
    Copy link
    Mannequin

    ash mannequin commented Aug 13, 2009

    I've added Tim Peters to the nosy list. Is there anyone else who should
    be considered as doctest maintainer? I've also checked further Python
    versions - a quick a look at latest doctest source shows that the
    problem is still there.

    There are some details (and a workaround) in Cython FAQ:
    http://wiki.cython.org/FAQ#HowcanIrundoctestsinCythoncode.28pyxfiles.29.3F

    @zware
    Copy link
    Member

    zware commented Jul 17, 2013

    In looking at test_decimal for bpo-16748, 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.

    @zware zware added the type-feature A feature request or enhancement label Jul 17, 2013
    @zware
    Copy link
    Member

    zware commented Aug 20, 2013

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

    @bitdancer
    Copy link
    Member

    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.

    @zware
    Copy link
    Member

    zware commented Nov 11, 2013

    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.

    @zware
    Copy link
    Member

    zware commented Nov 19, 2013

    Does this qualify as a new feature that needs to be in before beta 1?

    @ericsnowcurrently
    Copy link
    Member

    Larry: thoughts?

    @zware
    Copy link
    Member

    zware commented Nov 24, 2013

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 24, 2013

    New changeset 95dc3054959b by Zachary Ware in branch 'default':
    Issue bpo-3158: doctest can now find doctests in functions and methods
    http://hg.python.org/cpython/rev/95dc3054959b

    @zware
    Copy link
    Member

    zware commented Nov 24, 2013

    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.

    @zware zware self-assigned this Nov 24, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Nov 24, 2013

    New changeset 30b95368d253 by Zachary Ware in branch 'default':
    Issue bpo-3158: Relax new doctests a bit.
    http://hg.python.org/cpython/rev/30b95368d253

    @zware zware closed this as completed Nov 27, 2013
    @takluyver
    Copy link
    Mannequin

    takluyver mannequin commented Jan 28, 2014

    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.
    ipython/ipython#4892

    @zware
    Copy link
    Member

    zware commented Jan 28, 2014

    Does this patch fix things for you?

    @zware zware reopened this Jan 28, 2014
    @jtaylor
    Copy link
    Mannequin

    jtaylor mannequin commented Jan 28, 2014

    the patch seems to work for me in ipython.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 6, 2014

    New changeset c964b6b83720 by Zachary Ware in branch 'default':
    Issue bpo-3158: Provide a couple of fallbacks for in case a method_descriptor
    http://hg.python.org/cpython/rev/c964b6b83720

    @zware
    Copy link
    Member

    zware commented Feb 6, 2014

    Done.

    @zware zware closed this as completed Feb 6, 2014
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 11, 2014

    New changeset 8520e0ff8e36 by R David Murray in branch 'default':
    whatsnew: doctest finds tests in extension modules (bpo-3158)
    http://hg.python.org/cpython/rev/8520e0ff8e36

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants