Opened 9 years ago
Closed 9 years ago
#10143 closed enhancement (fixed)
Bring 2D plotting up to 100% doctest coverage (except plot.py)
Reported by: | kcrisman | Owned by: | mvngu |
---|---|---|---|
Priority: | minor | Milestone: | sage-4.6.2 |
Component: | documentation | Keywords: | |
Cc: | jason, mvngu | Merged in: | sage-4.6.2.alpha3 |
Authors: | Karl-Dieter Crisman | Reviewers: | Minh Van Nguyen, Geoffrey Ehrman |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
There are a few functions missing doctesting in sage/plot that aren't in plot3d/. Fixing this is the subject of this ticket. Doing it for plot.py largely is about GraphicsArray objects, so that is a separate ticket - #10144.
With the patches below, the rebuilt reference manual can be found at
http://mvngu.googlecode.com/hg/10143-plot/index.html
Apply:
Attachments (2)
Change History (21)
comment:1 Changed 9 years ago by
- Cc jason mvngu added
comment:2 Changed 9 years ago by
- Status changed from new to needs_review
Okay, this adds the plot doc stuff, and also adds the z=0
option for a couple of .plot3d()
methods. Needs review, should be fairly straightforward.
comment:3 Changed 9 years ago by
- Description modified (diff)
Changed 9 years ago by
comment:4 Changed 9 years ago by
- Description modified (diff)
- Reviewers set to Minh Van Nguyen
kcrisman's patch is fine by me. However, I noticed some misformatting and a broken link to the function show()
. These are fixed in my reviewer patch. Someone needs to look over that one. I noticed that the function ensure_subs()
is removed by kcrisman's patch. This is OK because that function is not used anywhere in the Sage library. Grepping through the whole Sage library only shows that ensure_subs()
is defined and imported, but never used. See the ticket description for instructions on which tickets to apply.
comment:5 Changed 9 years ago by
Thanks for fixing those - that's why we have reviews!
I thought I explained elsewhere on the ticket why I removed that function, but maybe I didn't? I came to the same conclusion you did; it was used in previous plot behavior, but no longer.
comment:6 Changed 9 years ago by
I gave kcrisman's patch a positive review. Someone just needs to review my patch for this ticket to be closed as fixed.
comment:7 Changed 9 years ago by
- Status changed from needs_review to positive_review
Minh's reviewer's patch looks good.
comment:8 Changed 9 years ago by
- Milestone changed from sage-4.6.1 to sage-4.6.2
comment:9 follow-up: ↓ 10 Changed 9 years ago by
- Status changed from positive_review to needs_work
This needs to be rebased to apply on top of #9907.
comment:10 in reply to: ↑ 9 Changed 9 years ago by
comment:11 Changed 9 years ago by
comment:12 Changed 9 years ago by
- Status changed from needs_work to needs_review
The only rebase needed was in removing the ensure_subs
function in misc.py. The reviewer patch still applies after this. There are no other changes, and I once again checked docs and tests, so I think positive review restoration is appropriate.
To buildbot: depends on #9781, #8236, and #10244 - apply trac_10143-2dplotcoverage.patch and trac-10143_reviewer.patch
comment:13 Changed 9 years ago by
- Reviewers changed from Minh Van Nguyen to Minh Van Nguyen, Geoffrey Ehrman
- Status changed from needs_review to positive_review
comment:14 Changed 9 years ago by
Just FYI - still applies fine on 4.6.2.alpha0.
comment:15 follow-ups: ↓ 16 ↓ 18 Changed 9 years ago by
comment:16 in reply to: ↑ 15 ; follow-up: ↓ 17 Changed 9 years ago by
- Description modified (diff)
comment:17 in reply to: ↑ 16 Changed 9 years ago by
Replying to jdemeyer:
Well, specifiying all dependencies is much more useful actually.
I completely agree, but it seems to confuse the buildbot which tried to apply patches twice.
comment:18 in reply to: ↑ 15 Changed 9 years ago by
The first of the current dependencies is closed, the second, I believe, is a mistake and should have been #8632, which is a prerequisite for #10244. So it seems that I can just say
Humble pie - BOTH the first two were typos. Sorry. The new description is correct.
Depends on #10244
I see what is going on. Useful for buildbot, but not at all useful for people manually applying patches who won't know how many iterations they have to go back! Thanks for the clarification.
comment:19 Changed 9 years ago by
- Merged in set to sage-4.6.2.alpha3
- Resolution set to fixed
- Status changed from positive_review to closed
Patch coming up soon (just checking that build docs still look correct).