Opened 7 years ago

Closed 19 months ago

#14272 closed defect (wontfix)

Add DocTestFinder to the doctesting framework

Reported by: roed Owned by: roed
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: doctest framework Keywords:
Cc: andrew.mathas, vdelecroix Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #12415 Stopgaps:

Description

We should use Python's DocTestFinder to generate doctests from Python files: our current approach misses some doctests that are added dynamically:

sage: from sage.doctest.sources import FileDocTestSource
sage: os.chdir(os.path.join(os.environ['SAGE_ROOT'],'devel','sage'))
sage: filename1 = 'sage/combinat/partition.py'
sage: FDS1 = FileDocTestSource(filename1, True, True, True, False)
sage: filename2 = 'sage/combinat/tableau.py'
sage: FDS2 = FileDocTestSource(filename2, True, True, True, False)
sage: FDS1._test_enough_doctests()
There are 18 tests in sage/combinat/partition.py that are not being run
    Tests on lines 315, 316, 318, 319, 321, 322, 328, 330, 332, 333, 338, 339, 348, 349, 352, 353, 358, 361 are not run
sage: FDS2._test_enough_doctests()
There are 12 tests in sage/combinat/tableau.py that are not being run
    Tests on lines 120, 121, 123, 124, 127, 128, 135, 136, 140, 141, 145, 148 are not run

Attachments (1)

test-python-doctests.py (727 bytes) - added by jdemeyer 7 years ago.
Testcases by nthiery

Download all attachments as: .zip

Change History (16)

comment:1 Changed 7 years ago by roed

  • Dependencies set to #12415

comment:2 Changed 7 years ago by roed

  • Component changed from doctest to doctest framework
  • Owner changed from mvngu to roed

comment:3 Changed 7 years ago by andrew.mathas

  • Cc andrew.mathas added

All of these doctests appear in a string which is passed as an argument to the GlobalOptions? class. For example, here is part of the "problem" in partition.py:

PartitionOptions=GlobalOptions(name='partitions',
    doc=r"""
    Sets and displays the global options for elements of the partition, skew
    partition, and partition tuple classes.  If no parameters are set, then the
    function returns a copy of the options dictionary.

    The ``options`` to partitions can be accessed as the method
    :obj:`Partitions.global_options` of :class:`Partitions` and
    related parent classes.
    """,
    end_doc=r"""
    EXAMPLES::

        sage: P = Partition([4,2,2,1])
        sage: P
        [4, 2, 2, 1]
        sage: Partitions.global_options(display="exp")
        sage: P
        1, 2^2, 4
        sage: Partitions.global_options(display="exp_high")
        sage: P
        4, 2^2, 1

*snip*

The string end_doc contains all of the untested doc-tests but it is an argument being passed to a class rather than a documentation string. This string will become part of the doc-string for the associated options classes for partitions (resp. tableaux), so doc-testing it would be a good idea.

Arguably, however, doc-tests should not be performed on hard coded strings in the code unless they are actually doc-strings. It is not clear to me how to fix this problem or even if it should be fixed.

comment:4 follow-up: Changed 7 years ago by roed

The approach advocated in this ticket would be to get these doctests from the __doc__ attribute of the class after parsing the file. I think that's what doctest.DocTestFinder does, but I haven't looked into the details so I'm not sure.

comment:5 in reply to: ↑ 4 Changed 7 years ago by andrew.mathas

Replying to roed:

The approach advocated in this ticket would be to get these doctests from the __doc__ attribute of the class after parsing the file. I think that's what doctest.DocTestFinder does, but I haven't looked into the details so I'm not sure.

Thanks for the explanation. I see that I didn't read the ticket very well...

Changed 7 years ago by jdemeyer

Testcases by nthiery

comment:6 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:7 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:8 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:9 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:10 Changed 5 years ago by vdelecroix

  • Cc vdelecroix added

Hello,

I independently opened this discussion on sage-devel. There are cases were Sage is too laxist as

class A:
    a = (
        """
        TESTS::

            sage: print "Is this a test?"
            Of course not!
        """
     )

the string above is considered as a doctest!!

Vincent

comment:11 Changed 2 years ago by jdemeyer

I feel like closing this ticket as "wontfix". First of all, the current doctester works quite well (except for GlobalOptions, but that can be fixed thanks to #23238). It is actually convenient that it works purely syntactically, there is no need to recompile anything to run changed doctests. The current approach also works well with things that don't have a docstring at all, like cdef functions in Cython.

And Vincent, I think that #23196 fixed your last comment.

comment:12 Changed 19 months ago by andrew.mathas

I agree with Jeronen. Shall we close this?

comment:13 Changed 19 months ago by roed

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

Fine with me.

comment:14 Changed 19 months ago by andrew.mathas

  • Status changed from needs_review to positive_review

comment:15 Changed 19 months ago by jdemeyer

  • Resolution set to wontfix
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.