Ticket #10589 (needs_review defect)

Opened 2 years ago

Last modified 5 days ago

sage -fixdoctests doesn't work correctly

Reported by: mderickx Owned by: mvngu
Priority: minor Milestone: sage-5.10
Component: scripts Keywords: days45
Cc: Work issues:
Report Upstream: N/A Reviewers:
Authors: Andrew Mathas Merged in:
Dependencies: #12415 Stopgaps:

Description (last modified by andrew.mathas) (diff)

This patch fixes the following problems with the command line fixdoctests option:

  • correctly deals with tests for which output it expected but none is generated, or for which no output is expected by some is generated.
  • makes fixdoctests play nicely with the changes introduced in #12415
  • adds some tests to sage/tests/cmdline.py
  • makes the script work slightly better with tests that produce output with varying indentation
  • cleans up the code of sage-fixdoctests

---

Original ticket:

To reproduce this remove all but the first test result from the patch at: #10568

maarten-derickxs-macbook-pro:sage maarten$ sage -t matrix/matrix_sparse.pyx
sage -t  "devel/sage-main/sage/matrix/matrix_sparse.pyx"    
**********************************************************************
File "/Applications/sage-4.6.rc0/devel/sage-main/sage/matrix/matrix_sparse.pyx", line 301:
    sage: (2/3)*M
Expected nothing
Got:
    [   0  2/3  4/3    2  8/3 10/3]
    [   4 14/3 16/3    6 20/3 22/3]
    [   8 26/3 28/3   10 32/3 34/3]
**********************************************************************
File "/Applications/sage-4.6.rc0/devel/sage-main/sage/matrix/matrix_sparse.pyx", line 302:
    sage: 7*M
Expected nothing
Got:
    [  0   7  14  21  28  35]
    [ 42  49  56  63  70  77]
    [ 84  91  98 105 112 119]
**********************************************************************
File "/Applications/sage-4.6.rc0/devel/sage-main/sage/matrix/matrix_sparse.pyx", line 303:
    sage: (1/4)*M
Expected nothing
Got:
    [   0  1/4  1/2  3/4    1  5/4]
    [ 3/2  7/4    2  9/4  5/2 11/4]
    [   3 13/4  7/2 15/4    4 17/4]
**********************************************************************
File "/Applications/sage-4.6.rc0/devel/sage-main/sage/matrix/matrix_sparse.pyx", line 306:
    sage: m==(97/42)*(42/97*m)
Expected nothing
Got:
    True
**********************************************************************
1 items had failures:
   4 of   9 in __main__.example_6
***Test Failed*** 4 failures.
For whitespace errors, see the file /Users/maarten/.sage//tmp/.doctest_matrix_sparse.py
	 [12.5 s]
 
----------------------------------------------------------------------
The following tests failed:


	sage -t  "devel/sage-main/sage/matrix/matrix_sparse.pyx"
Total time for all tests: 12.6 seconds
maarten-derickxs-macbook-pro:sage maarten$ sage -fixdoctests matrix/matrix_sparse.pyx

The bug was caused by the script not not allowing for the cases when either the expected or computed returns from the doctests were empty. The attached patch does this properly and also cleans up the script a little (well, a lot, actually).

Note that the script lives in $SAGE_ROOT/local/bin which is not part of the standard repository. The (new) doctests for the script live in sage/tests/cmdline.py

Apply: trac_10589--doctests_for_fixdoctests-am.patch

Apply to scripts repository: trac_10589--fixdoctest_failures-am.patch (modifies $SAGE_ROOT/local/bin)

Attachments

trac_10589--doctests_for_fixdoctests-am.patch Download (3.1 KB) - added by andrew.mathas 3 weeks ago.
Fixing a typo
trac_10589--fixdoctest_failures-am.patch Download (6.2 KB) - added by andrew.mathas 2 weeks ago.
Fixing another stray comment

Change History

comment:1 Changed 3 months ago by andrew.mathas

  • Keywords sage45 added
  • Status changed from new to needs_review
  • Description modified (diff)
  • Authors set to Andrew Mathas

comment:2 Changed 3 months ago by andrew.mathas

  • Description modified (diff)
  • Milestone set to sage-5.8

comment:3 Changed 3 months ago by andrew.mathas

  • Description modified (diff)

comment:4 Changed 3 months ago by andrew.mathas

  • Description modified (diff)

comment:5 Changed 3 months ago by andrew.mathas

  • Description modified (diff)

comment:6 follow-up: ↓ 8 Changed 3 months ago by jhpalmieri

To doctest this, you could add some tests to devel/sage/sage/tests/cmdline.py

comment:7 Changed 3 months ago by andrew.mathas

  • Keywords days45 added; sage45 removed

comment:8 in reply to: ↑ 6 Changed 3 months ago by andrew.mathas

Replying to jhpalmieri:

To doctest this, you could add some tests to devel/sage/sage/tests/cmdline.py

Thanks John. I will have a look at this and add some tests.

Last edited 3 months ago by andrew.mathas (previous) (diff)

comment:9 Changed 3 months ago by andrew.mathas

  • Status changed from needs_review to needs_work

comment:10 Changed 8 weeks ago by roed

  • Component changed from doctest to doctest framework

comment:11 Changed 3 weeks ago by andrew.mathas

  • Status changed from needs_work to needs_review
  • Dependencies set to 12415

Updated fixdoctests patch so that it works with the new doctest framework in #12415 and added some tests to sage/tests/cmdline.py for the script

comment:12 Changed 3 weeks ago by andrew.mathas

  • Description modified (diff)

Changed 3 weeks ago by andrew.mathas

Fixing a typo

comment:13 Changed 3 weeks ago by andrew.mathas

  • Description modified (diff)

comment:14 Changed 2 weeks ago by andrew.mathas

  • Description modified (diff)

comment:15 Changed 2 weeks ago by andrew.mathas

  • Description modified (diff)

comment:16 Changed 2 weeks ago by andrew.mathas

  • Description modified (diff)

comment:17 follow-up: ↓ 18 Changed 2 weeks ago by roed

Shouldn't that be the scripts repo?

comment:18 in reply to: ↑ 17 Changed 2 weeks ago by andrew.mathas

  • Description modified (diff)

Replying to roed:

Shouldn't that be the scripts repo?

Yes, thanks. Wasn't sure what the right name was, just knew where to find it...

Changed 2 weeks ago by andrew.mathas

Fixing another stray comment

comment:19 Changed 5 days ago by jdemeyer

  • Dependencies changed from 12415 to #12415
  • Component changed from doctest framework to scripts
Note: See TracTickets for help on using tickets.