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

Sage doctest framework ignores amendments by decorators #34828

Open
dimpase opened this issue Dec 6, 2022 · 10 comments
Open

Sage doctest framework ignores amendments by decorators #34828

dimpase opened this issue Dec 6, 2022 · 10 comments

Comments

@dimpase
Copy link
Member

dimpase commented Dec 6, 2022

In vanilla Python it's prefectly possible to generate docstrings of a class or a function with a decorator, they will be doctested just fine.

However, in Sage it's not so - such amendments are ignored. Example:
running sage -t c.py on the following file

# c.py
def docstr(f):
    """
    amends the docstring
    """
    f.__doc__ += """
    sage: len(tst.__subclasses__()) > 0
    True
    """
    return f

@docstr
class tst:
    '''testing
    sage: len(tst.__subclasses__()) <= 1
    True
    '''
    pass

if __name__ == '__main__':
    import doctest
    doctest.testmod()

results in

sage -t --warn-long 123.3 --random-seed=313143694260561478650367732388987074616 c1.py
    [1 test, 0.01 s]
----------------------------------------------------------------------
All tests passed!

If we modify c.py to correct prompts in doctests to vanilla Python,
and doctest:

$ sed -r "s/sage\:/>>>/" c1.py > ccc.py
$ python3 ccc.py 
**********************************************************************
File "/tmp/ccc.py", line 17, in __main__.tst
Failed example:
    len(tst.__subclasses__()) > 0
Expected:
    True
Got:
    False
**********************************************************************
1 items had failures:
   1 of   2 in __main__.tst
***Test Failed*** 1 failures.

we see that the doctest added by the decorator gets run (it fails by design, that's OK).

As we often have a lot of repeated boilerplate in docstrings, we're doing too much copy-paste, intstead of using decorators (it appears to be a common belief that doctest frameworks scrape code from files - it's not true, they operate on __doc__ attributes, as we see).

So something should be fixed in Sage doctest framework here.

CC: @mkoeppe @roed314 @robertwb @williamstein

Component: doctest framework

Issue created by migration from https://trac.sagemath.org/ticket/34828

@dimpase dimpase added this to the sage-9.8 milestone Dec 6, 2022
@dimpase
Copy link
Member Author

dimpase commented Dec 6, 2022

comment:2

cc'ing the authors of Sage's doctest framework

@dimpase

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:4

I would phrase this as an enhancement, not a defect. Sage's doctesting works by, as you say, scraping code from files, and you're asking that it instead read __doc__ attributes. Seems like a good idea, but I'm probably missing some subtleties.

Maybe William can fill in some of the history. I wonder if in Sage's early days, Python's doctesting framework was underdeveloped, or maybe even nonexistent, so Sage created its own? By the time David Roe updated the framework, he used some of Python's doctest module, and maybe now, ten years later, it's time to revisit the whole thing. I'm not volunteering, though.

@mkoeppe
Copy link
Member

mkoeppe commented Dec 6, 2022

comment:5

In the long run, I would hope that we can replace our own doctest discovery by a pytest plugin - see #33546 and the open follow-up tickets (such as #33826).

@roed314
Copy link
Contributor

roed314 commented Dec 6, 2022

comment:6

Sage's doctesting framework creates the tests by reading the file and searching for triple quoted strings; it does not actually execute python code. So I don't think there's an easy fix for this issue.

Here's the function that actually creates the doctests: https://github.com/sagemath/sagetrac-mirror/blob/develop/src/sage/doctest/sources.py#n230

@dimpase
Copy link
Member Author

dimpase commented Dec 6, 2022

comment:7

Replying to David Roe:

Sage's doctesting framework creates the tests by reading the file and searching for triple quoted strings; it does not actually execute python code. So I don't think there's an easy fix for this issue.

The only reason for this I can imagine is wanting one tool for Python and Cython.
For Python, it seems that one only needs to teach Python's doctest framework that the prompts in doctests are sage:, not >>>, no?

For Cython - I actually don't know why one can't run a preprocessor of your choice on Cython, before doctesting...

@roed314
Copy link
Contributor

roed314 commented Dec 6, 2022

comment:8

I'm not saying it's not possible to change how the tests are created. But that's not currently how we interact with Python's doctest module.

@roed314
Copy link
Contributor

roed314 commented Dec 6, 2022

comment:9

I'm not going to be able to look at this much anytime soon (giving a talk in mid January that I still have a TON of work to do for, and then multiple trips after that). I think if I was trying to add this capability to Sage's doctest framework, I would try to create a new subclass of DoctestSource that actually executed the file and then extracted tests from it. But I don't know what issues might arise when you try to do that.

@dimpase
Copy link
Member Author

dimpase commented Dec 7, 2022

comment:10

Replying to Matthias Köppe:

In the long run, I would hope that we can replace our own doctest discovery by a pytest plugin - see #33546 and the open follow-up tickets (such as #33826).

I tried sage --pytest c.py on examples with decorator as above and it worked just fine, like the vanilla Python doctests. This is still with >>> prompts. With sage: prompts everything was ignored - I think it's because it's out of Sage tree (is it a Sage bug?)

On the other hand, if on the git branch
u/dimpase/deprecate_sage_interfaces_is____element_functions, where I have src/sage/interfaces/abc.py with some decorator - generated docstrings, it all works:

$ ./sage --pytest src/sage/interfaces/abc.py 
===================================== test session starts ======================================
platform linux -- Python 3.9.2, pytest-7.1.3, pluggy-1.0.0
rootdir: /home/dimpase/work/software/sage/src, configfile: tox.ini
collected 4 items                                                                              

src/sage/interfaces/abc.py ....                                                          [100%]

====================================== 4 passed in 0.06s =======================================

4 is the total number of classes there, 2 with decorater-generated docstrings, and 2 with "normal" ones. Note that pytest counts everything in one docstring as one test - in this case anyway there is just one test per class.

@dimpase
Copy link
Member Author

dimpase commented Dec 7, 2022

comment:11

Opened #34829 to address the out of tree oddity noted in comment:10

@mkoeppe mkoeppe removed this from the sage-9.8 milestone Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants