#12491 closed enhancement (fixed)
Bring three more plot3d files to 100% coverage
Reported by:  kcrisman  Owned by:  jason, was 

Priority:  major  Milestone:  sage5.0 
Component:  graphics  Keywords:  
Cc:  Merged in:  sage5.0.beta9  
Authors:  KarlDieter Crisman  Reviewers:  David Loeffler, John Palmieri 
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  Stopgaps: 
Description (last modified by )
plot3d/plot3d.py: 83% (15 of 18) plot3d/list_plot3d.py: 25% (1 of 4) plot3d/parametric_plot3d.py: 20% (1 of 5) $ ../../sage coverage sage/plot/plot3d/list_plot3d.py  sage/plot/plot3d/list_plot3d.py SCORE sage/plot/plot3d/list_plot3d.py: 25% (1 of 4) Missing documentation: * list_plot3d_matrix(m, texture, **kwds): * list_plot3d_array_of_arrays(v, interpolation_type,texture, **kwds): * list_plot3d_tuples(v,interpolation_type, texture, **kwds): $ ./sage coverage devel/sage/sage/plot/plot3d/plot3d.py  devel/sage/sage/plot/plot3d/plot3d.py ERROR: Please add a `TestSuite(s).run()` doctest. SCORE devel/sage/sage/plot/plot3d/plot3d.py: 83% (15 of 18) Missing documentation: * triangle(self, a, b, c, color = None): * smooth_triangle(self, a, b, c, da, db, dc, color = None): * axes(scale=1, radius=None, **kwds): $ ../../sage coverage sage/plot/plot3d/parametric_plot3d.py  sage/plot/plot3d/parametric_plot3d.py SCORE sage/plot/plot3d/parametric_plot3d.py: 20% (1 of 5) Missing doctests: * _parametric_plot3d_curve(f, urange, plot_points, **kwds): * _parametric_plot3d_surface(f, urange, vrange, plot_points, boundary_style, **kwds): * adapt_if_symbolic(f): * adapt_to_callable(f, nargs=None):
Base ticket: #12024 bringing docs to 90%.
Attachments (2)
Change History (13)
comment:1 Changed 8 years ago by
 Description modified (diff)
 Summary changed from Bring two more plot3d files to 100% coverage to Bring three more plot3d files to 100% coverage
comment:2 Changed 8 years ago by
comment:3 Changed 8 years ago by
In addition, adapt_if_symbolic
, while not deprecated, has been stated to be for internal use since changeset 8271 and has not been used anywhere in Sage since changeset 8272  January, 2008, Sage 2.10.1.
So I'm going to remove it.
comment:4 Changed 8 years ago by
 Status changed from new to needs_review
It seems to pass all tests, and I looked at all the examples I added. Coverage is good. Needs review!
comment:5 Changed 8 years ago by
 Reviewers set to David Loeffler
Apply trac_12491.patch, trac_12491review.patch
(for the patchbot). This looks fine; I found no problems except a couple of minor typos and formatting fixes. Also I flagged some doctests with #indirect doctest
where necessary. Karl: if you're happy with my changes you can give this a positive review.
comment:6 Changed 8 years ago by
Apply trac_12491.patch, trac_12491review.patch
(for the patchbot, which spotted a trailing whitespace character in my previous patch)
comment:7 followup: ↓ 8 Changed 8 years ago by
I guess I try to do the most minor stuff possible. I don't have time to look at this right now, but I'll try to do so soon.
What's up with the indirect stuff? I assume this is for sage coverage
? I only check whether it has a doctest, not that stuff. Thanks.
comment:8 in reply to: ↑ 7 Changed 8 years ago by
 Status changed from needs_review to positive_review
Replying to kcrisman:
What's up with the indirect stuff? I assume this is for
sage coverage
?
Yes, exactly. The review patch looks good to me.
By the way, is it an issue that the instances of the _Coordinates
class don't seem to be picklable?
sage/plot/plot3d/plot3d.py ERROR: Please add a `TestSuite(s).run()` doctest. SCORE sage/plot/plot3d/plot3d.py: 100% (18 of 18)
But if I add a TestSuite
doctest, it fails: if I make this change:

sage/plot/plot3d/plot3d.py
diff git a/sage/plot/plot3d/plot3d.py b/sage/plot/plot3d/plot3d.py
a b class _Coordinates(object): 108 108 109 109 sage: from sage.plot.plot3d.plot3d import _ArbitraryCoordinates as 110 110 sage: x,y,z=var('x,y,z') 111 sage: arb((x+z,y*z,z), z, (x,y))111 sage: c=arb((x+z,y*z,z), z, (x,y)); c 112 112 Arbitrary Coordinates coordinate transform (z in terms of x, y) 113 sage: TestSuite(c).run() 113 114 """ 114 115 import inspect 115 116 all_vars=inspect.getargspec(self.transform).args[1:]
then I get this failure:
sage t "devel/sagemain/sage/plot/plot3d/plot3d.py" ********************************************************************** File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/sage5.0.beta7gcc/devel/sagemain/sage/plot/plot3d/plot3d.py", line 113: sage: TestSuite(c).run() Expected nothing Got: Failure in _test_pickling: Traceback (most recent call last): File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/sage5.0.beta7gcc/local/lib/python/sitepackages/sage/misc/sage_unittest.py", line 275, in run test_method(tester = tester) File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/sage5.0.beta7gcc/local/lib/python/sitepackages/sage/misc/sage_unittest.py", line 499, in _test_pickling tester.assertEqual(loads(dumps(self._instance)), self._instance) File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/sage5.0.beta7gcc/local/lib/python2.7/unittest/case.py", line 509, in assertEqual assertion_func(first, second, msg=msg) File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/sage5.0.beta7gcc/local/lib/python2.7/unittest/case.py", line 502, in _baseAssertEqual raise self.failureException(msg) AssertionError: Arbitrary Coordinates coordinate transform (z in terms of x, y) != Arbitrary Coordinates coordinate transform (z in terms of x, y)  The following tests failed: _test_pickling
A doctest like c == loads(dumps(c))
also fails. If this can be fixed, it probably should be, but on another ticket (unless it's trivial to do here).
comment:9 Changed 8 years ago by
 Reviewers changed from David Loeffler to David Loeffler, John Palmieri
Thanks, John. Definitely another ticket  this pickling stuff is always tricky.
comment:10 Changed 8 years ago by
 Merged in set to sage5.0.beta9
 Resolution set to fixed
 Status changed from positive_review to closed
comment:11 Changed 5 years ago by
 Description modified (diff)
Just some notes...
axes
appears to be used nowhere else, but since it is in a doctest, I won't remove it on this ticket. See also some of the other constructions in the various 3d plot directories which perform similar functions.adapt_to_callable
is deprecated, as is a piece in it where we use functioncall syntax. Given that this was deprecated in #7008 (and let me tell you, figuring that out was nontrivial!) and Sage4.1.2, from October 2009, I am going to remove this.