Opened 11 years ago
Closed 10 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:
- remove this functionality from local/bin/sage-*test*
- 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)
Change History (9)
comment:1 Changed 11 years ago by
Changed 10 years ago by
comment:2 Changed 10 years ago by
- 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 10 years ago by
comment:3 Changed 10 years ago by
- 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 10 years ago by
comment:4 Changed 10 years ago by
- 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 10 years ago by
- Resolution set to fixed
- Status changed from new to closed
Merged in Sage 3.3.alpha2
NOTE: Some people want a
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.