Opened 3 years ago
Closed 2 years ago
#12024 closed enhancement (fixed)
90% doctest coverage thrust metaticket
Reported by: | was | Owned by: | mvngu |
---|---|---|---|
Priority: | major | Milestone: | sage-5.8 |
Component: | doctest coverage | Keywords: | |
Cc: | kcrisman | Merged in: | |
Authors: | Reviewers: | Travis Scrimshaw | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by kcrisman)
After deleting the server directory (#11409) we need to add doctests to about 588 more functions to get coverage to 90%, which is a major goal for sage-5.0, which we've been working on getting to for over a year now. I did an audit and came up with about 350 functions in 18 files for which adding coverage will not be too hard.
Just edit the ticket description and add your name after the * that you will get coverage to 100% on that file. You an also create a ticket with your tests (put #number) in. Then it will be easy to referee all these doctest patches. Try to make different tickets if/when you find bugs, and restrict your ticket to just doctests so it is easy to referee. You can also add another file to the list.
Need Review / Need Work(243 functions to doctest, 25 functions to remove):
- misc/fpickle.pyx (#12192): 44% (4 of 9), easy -- Julian Rueth
- interfaces/psage.py (#12061): 85% (12 of 14), easy -- ppurka
- media/wav.py (#12673): 0% (0 of 25), delete: this is horribly buggy -- William Stein
- categories/action.pyx (#12039): 0% (0 of 31), harder -- David Roe
Claimed (155 functions to doctest):
- rings/laurent_series_ring* (#12259): 50% (38 of 76), easy -- David Roe
- rings/polynomial/polynomial_compiled.pyx (#12033): 0% (0 of 20), medium -- Tom Boothby
- rings/pari_ring.py (#12550): 15% (3 of 20), easy -- Frithjof Schulze
- misc/log.py (#12906): 0% (0 of 42), easy, just make object and call methods -- Itai Bar-Natan
- rings/power_series* (#14053): 63% (66 of 104), medium -- Kannappan Sampath
Unclaimed (102 functions to doctest, 21 functions to remove):
- matrix/matrix_window.pyx: 0% (0 of 26), medium, look at some code elsewhere that uses this, and just go through examples -- ?
- matrix/matrix_window_modn_dense.pyx: 0% (0 of 11), medium -- ?
- rings/integer.pyx: 90% (124 of 137), medium, since you have to document and test functions related to the integer pool -- ?
- rings/polynomial/convolution.py: 46% (6 of 13), medium, since all functions are documented but coming up with good tests could be tricky -- ?
- rings/fast_arith.pyx: 12% (1 of 8), harder, since there are additional cdef functions to document and test and you need to worry about overflow in arithmetic with C ints -- ?
Positive review, awaiting merge (31 functions doctested):
- rings/complex* (#13618): 90% (352 of 393), easy, since most of the missing functions are arithmetic or simple -- Travis Scrimshaw
- rings/homset.py (#13618): 10% (1 of 10), easy -- Travis Scrimshaw
- rings/ideal.py (#13618): 67% (35 of 52), easy, since there's a good amount of documentation already and the remaining functions are mostly simple -- Travis Scrimshaw
- rings/rational* (#13618): 87% (131 of 151), easy -- Travis Scrimshaw
- rings/real* (#13618): 89% (491 of 551) -- Travis Scrimshaw
- Everything else in rings (#13685): -- Travis Scrimshaw
Complete (393 functions doctested, 16 functions removed):
- databases/stein_watkins.py (#12092): 0% (0 of 15), easy -- Moritz Minzlaff
- misc/cython.py 37% (#11954): 38% (6 of 16), medium -- John Palmieri
- plot/plot3d/three files 63% (17 of 27), easy (#12491) -- Karl-Dieter Crisman
- rings/finite_rings/ext_pari.py (#12261): medium -- David Roe
- gsl/interpolation.pyx (#12036): 0% (0 of 9), easy -- William Stein
- databases/bz2Pickle.py (#12067): 0% (0 of 4), delete -- R. Andrew Ohana
- databases/gamma0wt2.py (#12066): 0% (0 of 7), delete -- R. Andrew Ohana
- algebras/free_algebra_quotient* (#12044): 4% (1 of 28), easy -- William Stein
- matrix/benchmark.py (#12034): 0% (0 of 29), easy -- William Stein
- monoids/monoid.py (#12025): 0% (0 of 3), easy -- William Stein
- plot/plot.py 76% (53 of 69), easy (#12495) -- Karl-Dieter Crisman
- algebras/free_algebra_quotient.py (#12593): 6% (1 of 16), easy, there is a big example in there. Just use it to construct similar examples everywhere else in the file -- ?
- matrix/matrix0.pyx (#12585): 87% (73 of 87), easy -- Hugh Thomas
- libs/mpmath/ext_main.pyx (#8791): 7% (8 of 111), medium -- Fredrick and Harald
- rings/qqbar.py (#12662): 45% (112 of 244), harder, there are a lot of functions to document -- David Loeffler
- rings/integer_ring.pyx: 77% (35 of 45), easy -- (#12496) Samuel Lelièvre
- rings/coerce_python.py (#12668): 0% (0 of 5), delete: this is not used -- David Loeffler
- coding/linear_code.py (#12893): 82% (52 of 63) -- Benjamin Jones
- coding/sd_codes.py (#12897): 20% (1 of 5) -- Benjamin Jones
- databases/db.py (#10353): 0% (0 of 21), delete -- François Bissey
Change History (81)
comment:1 Changed 3 years ago by was
- Description modified (diff)
comment:2 Changed 3 years ago by was
- Description modified (diff)
comment:3 follow-up: ↓ 5 Changed 3 years ago by fredrik.johansson
comment:4 Changed 3 years ago by was
- Description modified (diff)
comment:5 in reply to: ↑ 3 Changed 3 years ago by was
Replying to fredrik.johansson:
The file from hell is #8791
I've put a link above, added your patch to 8791 directly to avoid confusion, and set it to "needs review". Thanks! And we will consider reviewing this a very high priority.
comment:6 Changed 3 years ago by was
- Description modified (diff)
comment:7 Changed 3 years ago by was
- Description modified (diff)
comment:8 Changed 3 years ago by was
- Description modified (diff)
comment:9 Changed 3 years ago by was
- Description modified (diff)
comment:10 Changed 3 years ago by roed
- Description modified (diff)
comment:11 Changed 3 years ago by was
- Description modified (diff)
comment:12 Changed 3 years ago by jhpalmieri
- Description modified (diff)
comment:13 Changed 3 years ago by was
- Description modified (diff)
comment:14 Changed 3 years ago by roed
- Description modified (diff)
comment:15 Changed 3 years ago by jpang
- Description modified (diff)
comment:16 Changed 3 years ago by was
- Description modified (diff)
comment:17 Changed 3 years ago by ppurka
- Description modified (diff)
comment:18 Changed 3 years ago by ohanar
- Description modified (diff)
comment:19 Changed 3 years ago by minz
- Description modified (diff)
comment:20 Changed 3 years ago by saraedum
- Description modified (diff)
comment:21 Changed 3 years ago by kcrisman
- Cc kcrisman added
comment:22 Changed 3 years ago by saraedum
- Description modified (diff)
comment:23 Changed 3 years ago by roed
- Description modified (diff)
comment:24 Changed 3 years ago by roed
- Description modified (diff)
comment:25 Changed 3 years ago by roed
- Description modified (diff)
comment:26 Changed 3 years ago by roed
- Description modified (diff)
comment:27 Changed 3 years ago by roed
- Description modified (diff)
comment:28 Changed 3 years ago by roed
- Description modified (diff)
comment:29 Changed 3 years ago by kcrisman
- Milestone changed from sage-4.8 to sage-5.0
- Priority changed from major to blocker
Just as an interim update, from a sage-devel thread.
> This is also a good occasion to paste "sage -coverageall" output from > 5.0.prealpha0: > Overall weighted coverage score: 86.3% > Total number of functions: 28917 > We need 1056 more function to get to 90% coverage. > We need 2502 more function to get to 95% coverage. > We need 3658 more function to get to 99% coverage. Deleting sage/server (the old old notebook that can go away in 5.0, IIRC) gives us this: Overall weighted coverage score: 88.1% Total number of functions: 27936 We need 516 more function to get to 90% coverage. We need 1913 more function to get to 95% coverage. We need 3031 more function to get to 99% coverage.
I've also moved the milestone and made this a blocker for 5.0, since it has been one of the goals for several years. TBDFL or release manager can change this if that's not appropriate.
comment:30 Changed 3 years ago by was
TBDFL or release manager can change this if that's not appropriate.
+1 to making this a blocker. We only have 1.9% to go!
comment:31 Changed 3 years ago by kcrisman
- Description modified (diff)
comment:32 Changed 3 years ago by fschulze
- Description modified (diff)
comment:33 Changed 3 years ago by kcrisman
- Description modified (diff)
comment:34 Changed 3 years ago by kcrisman
- Description modified (diff)
comment:35 Changed 3 years ago by kcrisman
- Description modified (diff)
comment:36 Changed 3 years ago by kcrisman
- Description modified (diff)
comment:37 Changed 3 years ago by slelievre
- Description modified (diff)
comment:38 Changed 3 years ago by slelievre
- Description modified (diff)
comment:39 Changed 3 years ago by kcrisman
- Description modified (diff)
Changing the number left to doctest in each category in the description is annoying. If someone else wants to do that, please do!
comment:40 Changed 3 years ago by kcrisman
- Description modified (diff)
comment:41 Changed 3 years ago by kcrisman
- Description modified (diff)
comment:42 Changed 3 years ago by fschulze
- Description modified (diff)
comment:43 Changed 3 years ago by hthomas
- Description modified (diff)
comment:44 Changed 3 years ago by jlopez
- Description modified (diff)
comment:45 Changed 3 years ago by jlopez
- Description modified (diff)
I tried to claim algebras/free_algebra_quotient.py just to find out that it already has 100% doctest coverage in sage 5.0 beta 2.
Ticket #12593 can be closed as invalid.
comment:46 Changed 3 years ago by jhpalmieri
- Description modified (diff)
comment:47 Changed 3 years ago by ppurka
- Description modified (diff)
The two remaining functions which are not directly or indirectly doctested probably don't need doctesting.
comment:48 Changed 3 years ago by hthomas
- Description modified (diff)
comment:49 Changed 3 years ago by hthomas
- Description modified (diff)
comment:50 Changed 3 years ago by kcrisman
- Description modified (diff)
comment:51 Changed 3 years ago by davidloeffler
- Description modified (diff)
comment:52 Changed 3 years ago by davidloeffler
- Description modified (diff)
comment:53 Changed 3 years ago by jhpalmieri
- Description modified (diff)
comment:54 Changed 3 years ago by davidloeffler
- Description modified (diff)
comment:55 Changed 3 years ago by davidloeffler
- Description modified (diff)
comment:56 Changed 3 years ago by roed
- Description modified (diff)
comment:57 Changed 3 years ago by davidloeffler
- Description modified (diff)
comment:58 Changed 3 years ago by davidloeffler
- Description modified (diff)
comment:59 Changed 3 years ago by AlexGhitza
- Description modified (diff)
comment:60 Changed 3 years ago by jdemeyer
Current status in the hopefully soon-to-be-released sage-5.0.beta12:
Overall weighted coverage score: 88.1% Total number of functions: 29436 We need 551 more function to get to 90% coverage.
comment:61 Changed 3 years ago by kcrisman
What will that be once #11409 is deleted?
comment:62 Changed 3 years ago by davidloeffler
Taking a vanilla beta11 and installing #11409 plus a few patches which will be in beta12 (#7926, #12747 and #12749) we get:
Overall weighted coverage score: 89.9% Total number of functions: 28475 We need 25 more function to get to 90% coverage.
So close! Sadly #11409 seems to be in "needs work" limbo at the moment (the Sage library patch there doesn't even install cleanly any more -- there is a reject in all.py).
comment:63 follow-up: ↓ 64 Changed 3 years ago by jdemeyer
- Priority changed from blocker to major
Going for 90% is a nice goal, but I don't think it should block the sage-5.0 release.
In sage-5.0.beta13:
{{{{
Overall weighted coverage score: 88.3%
Total number of functions: 29374
We need 494 more function to get to 90% coverage.
We need 1963 more function to get to 95% coverage.
We need 3138 more function to get to 99% coverage.
}}}
comment:64 in reply to: ↑ 63 Changed 3 years ago by was
Replying to jdemeyer:
Going for 90% is a nice goal, but I don't think it should block the sage-5.0 release.
Agreed. We are close, and we will get there, but not for sage-5.0.
In sage-5.0.beta13:
{{{{
Overall weighted coverage score: 88.3%
Total number of functions: 29374
We need 494 more function to get to 90% coverage.
We need 1963 more function to get to 95% coverage.
We need 3138 more function to get to 99% coverage.
}}}
comment:65 Changed 3 years ago by davidloeffler
We'd be there already if #11409 went in, wouldn't we?
comment:66 Changed 3 years ago by benjaminfjones
- Description modified (diff)
comment:67 Changed 3 years ago by benjaminfjones
- Description modified (diff)
comment:69 Changed 3 years ago by tscrim
- Description modified (diff)
Claiming files and moving closed tickets.
comment:70 Changed 3 years ago by jhpalmieri
This is perhaps cheating, but I wonder if we should revise the coverage script to ignore all files in any directory containing a file "nodoctest.py". This would eliminate all files in the server directory from the coverage count.
comment:71 Changed 3 years ago by tscrim
- Description modified (diff)
Moving #13618 to need review block. Re-tallied function count.
comment:72 Changed 2 years ago by tscrim
- Description modified (diff)
I also did rings/real* in #13618.
comment:73 Changed 2 years ago by chapoton
Ticket #12092 is ready for review and should be easy enough.
comment:74 Changed 2 years ago by knsam
- Description modified (diff)
comment:75 Changed 2 years ago by knsam
- Description modified (diff)
comment:76 Changed 2 years ago by jdemeyer
You might also be interested in #14061, which fixes the sage-coverage script and brings the results for sage-5.7.beta3 to
Overall weighted coverage score: 89.8% Total number of functions: 31279 We need 48 more functions to get to 90% coverage. We need 1612 more functions to get to 95% coverage. We need 2863 more functions to get to 99% coverage.
comment:77 Changed 2 years ago by tscrim
- Description modified (diff)
comment:78 Changed 2 years ago by tscrim
- Description modified (diff)
comment:79 Changed 2 years ago by tscrim
- Description modified (diff)
comment:80 Changed 2 years ago by kcrisman
- Description modified (diff)
comment:81 Changed 2 years ago by jdemeyer
- Resolution set to fixed
- Reviewers set to Travis Scrimshaw
- Status changed from new to closed
The file from hell is #8791