Opened 7 years ago

Closed 7 years ago

#16746 closed defect (fixed)

Use the Sage displayhook in doctests

Reported by: vbraun Owned by:
Priority: blocker Milestone: sage-6.4
Component: doctest framework Keywords:
Cc: dkrenn Merged in:
Authors: Volker Braun Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: 3070715 (Commits, GitHub, GitLab) Commit: 3070715a7f656326260a9ef6635884e69fa0136f
Dependencies: #16992 Stopgaps:

Status badges

Description (last modified by vbraun)

The doctests output should look like the Sage commandline output, which it currently does not quite. Differences include:

  • Dictionaries will be sorted by key, makes output more reproducable
  • Sets are printed as {1,2} instead of set([1,2])
  • Empty set is set() instead of set([])
  • Types are <Foo at 0x...> instead of <Foo object at 0x...>

Other changes:

  • Normal tracebacks can now be distinguished from tracebacks in __repr__ (IPython extension):
    sage: f(*args)
    <repr(<sage.interfaces.gap.GapElement at 0x...>) failed: ValueError: The session in which this object was defined is no longer running.>
    
  • Plots now print themselves in doctest mode:
    sage: A.plot()
    Graphics object consisting of 1 graphics primitive
    

Change History (40)

comment:1 Changed 7 years ago by vbraun

  • Branch set to u/vbraun/sort_dicts_in_doctests

comment:2 Changed 7 years ago by vbraun

  • Authors set to Volker Braun
  • Commit set to 338c5c30ebfbd9a535cd291fc679625f4ab86164
  • Component changed from PLEASE CHANGE to doctest framework
  • Type changed from PLEASE CHANGE to defect

New commits:

338c5c3Use Sage display hook in doctests

comment:3 Changed 7 years ago by vbraun

  • Description modified (diff)
  • Summary changed from Sort dicts in doctests to Use the Sage displayhook in doctests

comment:4 Changed 7 years ago by vbraun

  • Description modified (diff)

comment:5 Changed 7 years ago by vbraun

The first commit changes the doctest displayhook to the one we want. This causes various doctest changes that need to be fixed.

comment:6 Changed 7 years ago by dkrenn

  • Cc dkrenn added

comment:7 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:8 Changed 7 years ago by vbraun

I noticed that the fix for printing types "our way" (#14466) was incomplete, inside lists it still uses IPython style:

sage: type(1)
<type 'sage.rings.integer.Integer'>
sage: [type(1)]
[sage.rings.integer.Integer]

comment:9 Changed 7 years ago by git

  • Commit changed from 338c5c30ebfbd9a535cd291fc679625f4ab86164 to d3635588ff9758997ddde5729bba2b3f5ce44a1b

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

8c5f2b0Directly overwrite with fixed doctests
da59f20call git directly and from the right directory
e397b3eAdd newline at end of file and fix cmdline help
9135b6cMerge #16992 (Directly apply --fixdoctests) branch
89945c9Fix printing of types
d363558Refactor our displayhook IPython integration

comment:10 Changed 7 years ago by vbraun

  • Dependencies set to #16992

comment:11 Changed 7 years ago by vbraun

I've refactored our displayhook machinery so we can pick & choose which parts we want to take from IPython. Now works ok, still have to fixup old doctests.

comment:12 Changed 7 years ago by git

  • Commit changed from d3635588ff9758997ddde5729bba2b3f5ce44a1b to b579b925651aae99d40731436be2300f11238265

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

1b99c68Also fix the cmdline doctest
bdbdd5bMerge #16992 (Directly apply --fixdoctests) branch
a246e17Fix printing of types
eb6a66bRefactor our displayhook IPython integration
b579b92Update doctests that depended on the old displayhook

comment:13 Changed 7 years ago by vbraun

  • Description modified (diff)
  • Status changed from new to needs_review

comment:14 Changed 7 years ago by vbraun

  • Priority changed from major to blocker

comment:15 Changed 7 years ago by kcrisman

Wow, that is a massive set of changes... just out of curiosity, what does the documentation now look like for graphics? I don't know whether it is more obvious before or after this change that one needs to evaluate to get them. One reason for the spaces before (I mean the ones after plot doctests) was to "leave room" in the (live) doc for evaluating these.

comment:16 Changed 7 years ago by jdemeyer

I see why you needed #16992 now...

comment:17 Changed 7 years ago by jdemeyer

  • Status changed from needs_review to needs_work

In src/sage/graphs/generic_graph.py, this was probably intended as a doctest (although certainly something went wrong around those lines):

sage: g.layout(layout="acyclic_dummy", save_po=True)
{('0', 0): [0.3356712660780875, 0],
 ('0', 1): [0.33563875939602245, 1],
 ('1', 0): [0.6751346747788185, 0],
 ('1', 1): [0.6715689173983604, 1]}

comment:18 follow-up: Changed 7 years ago by vbraun

Honestly, I don't know what the g.layout(layout="acyclic_dummy", save_po=True) is supposed to do. I took out the 1+1 == 2 but kept the layout call since the latter changes the rng (so without it all following doctests change).

comment:19 Changed 7 years ago by jdemeyer

There is a problem with graphics objects: they used to be actually "plotted" by doctests to some random file, now this gone. To easily check this, execute

$ chmod 0000 local/bin/tachyon
$ ./sage -t src/sage/plot/circle.py

Without this patch, this breaks 4 doctests. With this patch, this only breaks 1 doctest (where an explicit .show() was given), meaning there are now 3 plots no longer tested.

comment:20 in reply to: ↑ 18 Changed 7 years ago by jdemeyer

Replying to vbraun:

Honestly, I don't know what the g.layout(layout="acyclic_dummy", save_po=True) is supposed to do. I took out the 1+1 == 2 but kept the layout call since the latter changes the rng (so without it all following doctests change).

I didn't mean to remove the call, just to show the output, i.e. remove the x =.

comment:21 Changed 7 years ago by jdemeyer

Apart from the issues in the last comments, the changes to the doctests look okay. I haven't looked at the new displayhook code though.

What does "repl" stand for by the way? Perhaps a one-paragraph explanation in src/sage/repl/all.py would be good.

comment:22 Changed 7 years ago by vbraun

Read-Eval-Print-Loop.

I'll also call show() on graphics in the doctest displayhook...

comment:23 Changed 7 years ago by cheuberg

make ptestlong on my linux mint 64 bit desktop gave a few failing doctests, mostly because sorting of integers and strings is more or less random.

sage -t --long src/doc/de/tutorial/programming.rst
**********************************************************************
File "src/doc/de/tutorial/programming.rst", line 503, in doc.de.tutorial.programming
Failed example:
    X
Expected:
    {1, 19, 'a'}
Got:
    {'a', 1, 19}
**********************************************************************
1 item had failures:
   1 of 143 in doc.de.tutorial.programming
    [113 tests, 1 failure, 1.61 s]
sage -t --long src/doc/en/tutorial/programming.rst
**********************************************************************
File "src/doc/en/tutorial/programming.rst", line 485, in doc.en.tutorial.programming
Failed example:
    X
Expected:
    {1, 19, 'a'}
Got:
    {'a', 1, 19}
**********************************************************************
1 item had failures:
   1 of 144 in doc.en.tutorial.programming
    [114 tests, 1 failure, 0.17 s]
sage -t --long src/doc/en/thematic_tutorials/tutorial-programming-python.rst
**********************************************************************
File "src/doc/en/thematic_tutorials/tutorial-programming-python.rst", line 707, in doc.en.thematic_tutorials.tutorial-programming-python
Failed example:
    d
Expected:
    {3/2: 17, 3: 17, 'key': [4, 1, 5, 2, 3], (3, 1, 2): 'goo'}
Got:
    {'key': [4, 1, 5, 2, 3], (3, 1, 2): 'goo', 3/2: 17, 3: 17}
**********************************************************************
File "src/doc/en/thematic_tutorials/tutorial-programming-python.rst", line 734, in doc.en.thematic_tutorials.tutorial-programming-python
Failed example:
    d
Expected:
    {3/2: 17, 3: 17, 10: 'a', 'key': [4, 1, 5, 2, 3], (3, 1, 2): 'goo'}
Got:
    {'key': [4, 1, 5, 2, 3], (3, 1, 2): 'goo', 3/2: 17, 3: 17, 10: 'a'}
**********************************************************************
File "src/doc/en/thematic_tutorials/tutorial-programming-python.rst", line 767, in doc.en.thematic_tutorials.tutorial-programming-python
Failed example:
    d
Expected:
    {3: 14, 10: 'newvalue', 20: 'newervalue', 'a': [1, 2, 3]}
Got:
    {'a': [1, 2, 3], 3: 14, 10: 'newvalue', 20: 'newervalue'}
**********************************************************************
1 item had failures:
   3 of 159 in doc.en.thematic_tutorials.tutorial-programming-python
    [158 tests, 3 failures, 0.16 s]
sage -t --long src/doc/fr/tutorial/programming.rst
**********************************************************************
File "src/doc/fr/tutorial/programming.rst", line 501, in doc.fr.tutorial.programming
Failed example:
    X
Expected:
    {1, 19, 'a'}
Got:
    {'a', 1, 19}
**********************************************************************
1 item had failures:
   1 of 143 in doc.fr.tutorial.programming
    [113 tests, 1 failure, 0.17 s]
sage -t --long src/doc/ru/tutorial/programming.rst
**********************************************************************
File "src/doc/ru/tutorial/programming.rst", line 469, in doc.ru.tutorial.programming
Failed example:
    X
Expected:
    {1, 19, 'a'}
Got:
    {'a', 1, 19}
**********************************************************************
1 item had failures:
   1 of 143 in doc.ru.tutorial.programming
    [113 tests, 1 failure, 0.17 s]
sage -t --long local/lib/python2.7/site-packages/sagenb-0.10.8.2-py2.7.egg/sagenb/notebook/worksheet.py
**********************************************************************
File "local/lib/python2.7/site-packages/sagenb-0.10.8.2-py2.7.egg/sagenb/notebook/worksheet.py", line 4063, in sagenb.notebook.worksheet.Worksheet.delete_all_output
Failed example:
    W.cell_list()
Expected:
    [Cell 0: in=2+3, out=
    5, Cell 1: in=open('afile', 'w').write('some text')
    print 'hello', out=
    ]
Got:
    [Cell 0: in=2+3, out=
     5, Cell 1: in=open('afile', 'w').write('some text')
     print 'hello', out=]
**********************************************************************
1 item had failures:
   1 of  21 in sagenb.notebook.worksheet.Worksheet.delete_all_output
    [530 tests, 1 failure, 13.40 s]

comment:24 Changed 7 years ago by git

  • Commit changed from b579b925651aae99d40731436be2300f11238265 to a9271f0de704f3919b58e0b22afa9d38a39c47d1

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

1a2b1cfAdd IPython formatting to the reference manual
48a2320Also call graphics.show() in doctest mode
fbf83ebShow output of the lonely generic_graph doctest
a9271f0Fix doctests that have dictionary keys without stable order

comment:25 Changed 7 years ago by vbraun

  • Status changed from needs_work to needs_review

I've addressed all issues so far and added more documentation (which also spells out REPL).

comment:26 Changed 7 years ago by vbraun

Found some more random failures. Also, missed the W.cell_list() failure above which is caused by not preserving a trailing newline (IPython does that, too). Fix coming shortly...

comment:27 Changed 7 years ago by git

  • Commit changed from a9271f0de704f3919b58e0b22afa9d38a39c47d1 to b362aa6c4ee2db82dc49ab42d52042cf9a46074a

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

3922ed5Reduce doctest precision
b362aa6Display trailing newline in __repr__() output

comment:28 Changed 7 years ago by git

  • Commit changed from b362aa6c4ee2db82dc49ab42d52042cf9a46074a to 7de159aef22abafd93bfccd8557065c7d802031f

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

7de159aFix more random test failures

comment:29 Changed 7 years ago by cheuberg

Now, all doctests pass on my linux mint 64 bit desktop.

comment:30 Changed 7 years ago by vbraun

All doctest pass on the buildbot...

comment:31 Changed 7 years ago by dimpase

Did you try this on ARM?

comment:32 Changed 7 years ago by vbraun

ARM is part of the buildbot now, so yes ;-)

comment:33 Changed 7 years ago by fbissey

  • Status changed from needs_review to positive_review

Since it pass on the build bot and I don;t see any obvious problem with the new code itself I will give it a positive review. I expect ARM to be irrelevant in this context as we are only talking about re-formatting current doctests. Plus it will fix broken doctests in sage-on-gentoo (dictionaries displayed in a different order).

Last edited 7 years ago by fbissey (previous) (diff)

comment:34 Changed 7 years ago by fbissey

  • Reviewers set to François Bissey

comment:35 Changed 7 years ago by git

  • Commit changed from 7de159aef22abafd93bfccd8557065c7d802031f to f9af4eed6fcf732ed815e83d9e2c8dd9cce08ca4
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

a25e286trac #17013: new class WordDatatype_char
398b33ctrac #17013: added is_square in WordDatatype_char
a563343trac #17013: doctests improvements + bug fix
c00964dtrac #17013: fix Python C API declaration
f9af4eeMerge #17013 to resolve conflict

comment:36 Changed 7 years ago by vbraun

  • Status changed from needs_review to positive_review

surprisingly only one merge conflict ;-)

comment:37 Changed 7 years ago by git

  • Commit changed from f9af4eed6fcf732ed815e83d9e2c8dd9cce08ca4 to 3070715a7f656326260a9ef6635884e69fa0136f
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

3070715fix another doctest

comment:38 Changed 7 years ago by fbissey

At this stage I think you have done all the work that should be done in one ticket. I suggest we have a follow up ticket for any new discoveries.

comment:39 Changed 7 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:40 Changed 7 years ago by vbraun

  • Branch changed from u/vbraun/sort_dicts_in_doctests to 3070715a7f656326260a9ef6635884e69fa0136f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.