Opened 4 years ago

Closed 4 years ago

#26038 closed defect (fixed)

Problem with doctesting framework and multiple lines

Reported by: jhpalmieri Owned by:
Priority: critical Milestone: sage-8.4
Component: doctest framework Keywords:
Cc: Merged in:
Authors: Jeroen Demeyer Reviewers: Erik Bray
Report Upstream: N/A Work issues:
Branch: d6f50b9 (Commits, GitHub, GitLab) Commit: d6f50b9c8e0912caddb25a844911fa0ca98c9302
Dependencies: #26037 Stopgaps:

Status badges

Description (last modified by jdemeyer)

The file geometry/polyhedron/backend_ppl.py contains the following doctest (basically at the end of the file):

        sage: p = Polyhedron(vertices=[(0,0),(1,0),(0,1)], rays=[(1,1)], lines=[])
        ....:                backend='ppl', base_ring=ZZ)
        sage: TestSuite(p).run(skip='_test_pickling')

This passes when using Sage built with Python 2, while Sage built with Python 3 says, quite correctly,

SyntaxError: multiple statements found while compiling a single statement

The first line p = Polyhedron(...) is a complete statement because of a typo – the ) instead of , at the end of that line – so the continuation line makes no sense. Why does this doctest pass? There must be a bug in Python.

Change History (15)

comment:1 Changed 4 years ago by jhpalmieri

I've marked this critical because I don't know if it is affecting other doctests, maybe causing lines to be ignored, the wrong doctests to be run, etc. I'm tempted to make it a blocker: something wrong with doctesting is serious.

comment:2 follow-up: Changed 4 years ago by jdemeyer

It's a Python issue. It seems that the single-statement parser literally parses a single statement and ignores anything else. This just works:

sage: compile("a = 1\n!@#$%^&*()", "", "single")

comment:3 in reply to: ↑ 2 Changed 4 years ago by jhpalmieri

Replying to jdemeyer:

It's a Python issue.

Does that mean we ignore it? Or is there a good way to catch the syntax error?

comment:4 Changed 4 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Dependencies set to #26037

comment:5 Changed 4 years ago by jdemeyer

  • Branch set to u/jdemeyer/problem_with_doctesting_framework_and_multiple_lines

comment:6 Changed 4 years ago by jdemeyer

  • Commit set to 3abd3a24aebdeed34c92f6c807032bb993ef86bd
  • Status changed from new to needs_review

This is the basic idea. I haven't ran all Sage doctests with this, it would be interesting to see if there are more problems like this.


New commits:

3b381f1trac 26037: fix typo in geometry/polyhedron/backend_ppl.py
3abd3a2Check that the doctest is a valid single statement

comment:7 Changed 4 years ago by jdemeyer

  • Description modified (diff)

comment:8 Changed 4 years ago by jhpalmieri

I get one doctest failure:

sage -t --long --warn-long 60.5 src/sage/repl/ipython_extension.py
**********************************************************************
File "src/sage/repl/ipython_extension.py", line 350, in sage.repl.ipython_extension.SageMagics.cython
Failed example:
    shell.run_cell('''
    %%cython
    def f():
        print('test')
    ''')
    shell.run_cell('f()')
Exception raised:
    Traceback (most recent call last):
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 650, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1059, in compile_and_execute
        compiled = compiler(example)
      File "/Users/jpalmier/Desktop/Sage/git/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 636, in compiler
        raise SyntaxError("doctest is not a single statement")
    SyntaxError: doctest is not a single statement
**********************************************************************

comment:9 Changed 4 years ago by jhpalmieri

The doctest is certainly not a single statement. If it should instead be

  • src/sage/repl/ipython_extension.py

    diff --git a/src/sage/repl/ipython_extension.py b/src/sage/repl/ipython_extension.py
    index 393bf95d2f..b5ea9e983d 100644
    a b class SageMagics(Magics): 
    352352            ....: def f():
    353353            ....:     print('test')
    354354            ....: ''')
    355             ....: shell.run_cell('f()')
     355            sage: shell.run_cell('f()')
     356            test
    356357        """
    357358        from sage.misc.cython import cython_compile
    358359        return cython_compile(cell)

that doesn't work for me when doctesting (NameError: name 'f' is not defined), although it does work interactively (well, interactively I also see some other information: <ExecutionResult object at ...> for both calls to shell.run_cell(...)). Is it a bug if it doesn't work in doctest mode? Or should it instead be

  • src/sage/repl/ipython_extension.py

    diff --git a/src/sage/repl/ipython_extension.py b/src/sage/repl/ipython_extension.py
    index 393bf95d2f..779d83ffd3 100644
    a b class SageMagics(Magics): 
    352352            ....: def f():
    353353            ....:     print('test')
    354354            ....: ''')
    355             ....: shell.run_cell('f()')
     355            sage: f()
     356            test
    356357        """
    357358        from sage.misc.cython import cython_compile
    358359        return cython_compile(cell)

?

comment:10 Changed 4 years ago by git

  • Commit changed from 3abd3a24aebdeed34c92f6c807032bb993ef86bd to d6f50b9c8e0912caddb25a844911fa0ca98c9302

Branch pushed to git repo; I updated commit sha1. New commits:

d6f50b9Fix %%cython doctest

comment:11 Changed 4 years ago by embray

I've seen this problem before too, but couldn't quite articulate it--John's example made it clear though and this fix makes sense.

comment:12 Changed 4 years ago by embray

I knew something about that test looked familiar: #25212

comment:13 Changed 4 years ago by jdemeyer

Any reviewers?

comment:14 Changed 4 years ago by embray

  • Reviewers set to Erik Bray
  • Status changed from needs_review to positive_review

I seem to recall being told that by changing that %%cython doctest I was defeating the purpose of the test. But I guess now you agree with my reasoning.

comment:15 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/problem_with_doctesting_framework_and_multiple_lines to d6f50b9c8e0912caddb25a844911fa0ca98c9302
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.