Opened 11 years ago

Closed 11 years ago

#4588 closed defect (fixed)

[with patch; positive review] doctest -- get rid of the "feature" where docstrings with require, optional, and package all in them are automatically marked optional

Reported by: was Owned by: mabshoff
Priority: major Milestone: sage-3.3
Component: doctest coverage Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

This "feature" that I introduced years ago is incredibly confusing. 3 people at least got very confused by this in the last 3-4 days.

To close this ticket:

  1. remove this functionality from local/bin/sage-*test*
  1. rewrite all the files that use this by tediously marking each optional line with #optional. This is tedious, but it is much much clearer what is going on.

Note -- only do this *after* apply #4583, which already does some of part 2 above.

Attachments (4)

trac_4588-scripts.patch (1.2 KB) - added by was 11 years ago.
trac_4588-sage.patch (109.6 KB) - added by was 11 years ago.
fix fallout in the core library.
trac_4588-part2-sage.patch (12.5 KB) - added by was 11 years ago.
trac_4588-part3-sage.patch (1.4 KB) - added by was 11 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 11 years ago by was

NOTE: Some people want a

# start optional
...
# end optional

system to allow for optional blocks.

I'm not sure. I think I don't like this.

One way to implement this though would be in sage-doctest when parsing the docstring if start optional appears, just mark everything # optional through to where end optional appears. I guess.

Changed 11 years ago by was

comment:2 Changed 11 years ago by was

  • Milestone changed from sage-3.4.1 to sage-3.3
  • Summary changed from doctest -- get rid of the "feature" where docstrings with require, optional, and package all in them are automatically marked optional to [with patch; not ready for review] doctest -- get rid of the "feature" where docstrings with require, optional, and package all in them are automatically marked optional

Fallout after applying the patch:

----------------------------------------------------------------------

The following tests failed:

        sage -t  devel/sage/sage/schemes/elliptic_curves/sha_tate.py # 1 doctests failed
        sage -t  devel/sage/sage/rings/polynomial/multi_polynomial_ideal.py # 1 doctests failed
        sage -t  devel/sage/sage/interfaces/matlab.py # 1 doctests failed
        sage -t  devel/sage/sage/interfaces/macaulay2.py # 1 doctests failed
        sage -t  devel/sage/sage/interfaces/octave.py # 2 doctests failed
        sage -t  devel/sage/sage/interfaces/scilab.py # 5 doctests failed
        sage -t  devel/sage/sage/interfaces/lie.py # 1 doctests failed
        sage -t  devel/sage/sage/interfaces/kash.py # 100 doctests failed
        sage -t  devel/sage/sage/interfaces/maple.py # 40 doctests failed
        sage -t  devel/sage/sage/interfaces/mupad.py # 19 doctests failed
        sage -t  devel/sage/sage/interfaces/qepcad.py # 66 doctests failed
        sage -t  devel/sage/sage/combinat/designs/incidence_structures.py # 1 doctests failed
        sage -t  devel/sage/sage/databases/sloane.py # 3 doctests failed
        sage -t  devel/sage/sage/databases/jones.py # 4 doctests failed
        sage -t  devel/sage/sage/databases/stein_watkins.py # 21 doctests failed
        sage -t  devel/sage/sage/groups/perm_gps/permgroup.py # 1 doctests failed
        sage -t  devel/sage/sage/graphs/graph_database.py # 1 doctests failed
        sage -t  devel/sage/sage/coding/linear_code.py # 4 doctests failed
----------------------------------------------------------------------
Total time for all tests: 171.8 seconds

Changed 11 years ago by was

fix fallout in the core library.

Changed 11 years ago by was

comment:3 Changed 11 years ago by was

  • Summary changed from [with patch; not ready for review] doctest -- get rid of the "feature" where docstrings with require, optional, and package all in them are automatically marked optional to [with patch; needs review] doctest -- get rid of the "feature" where docstrings with require, optional, and package all in them are automatically marked optional

The attached sage patches fix all missing # optionals after applying the scripts patch (the first one -- trac_4588-scripts.patch). I also greatly improve the use of

  # optional -- name_of_package

while I was at it.

However, note that this revealed some bugs in David Joyner's linear_code.py stuff. See #5067. Thus I believe this patch should receive a positive review *despite* that after applying it suddenly four doctests will fail. I've made #5067 a blocker.

Changed 11 years ago by was

comment:4 Changed 11 years ago by rlm

  • Summary changed from [with patch; needs review] doctest -- get rid of the "feature" where docstrings with require, optional, and package all in them are automatically marked optional to [with patch; positive review] doctest -- get rid of the "feature" where docstrings with require, optional, and package all in them are automatically marked optional

I think that this set of patches should be merged in. They all applied cleanly to alpha0 (I don't know about alpha1), and this definitely needs to be done. I think that any remaining issues which arise once this does get merged should become their own ticket.

comment:5 Changed 11 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged in Sage 3.3.alpha2

Note: See TracTickets for help on using tickets.