Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring three more plot3d files to 100% coverage #12491

Closed
kcrisman opened this issue Feb 10, 2012 · 16 comments
Closed

Bring three more plot3d files to 100% coverage #12491

kcrisman opened this issue Feb 10, 2012 · 16 comments

Comments

@kcrisman
Copy link
Member

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%.

Component: graphics

Author: Karl-Dieter Crisman

Reviewer: David Loeffler, John Palmieri

Merged: sage-5.0.beta9

Issue created by migration from https://trac.sagemath.org/ticket/12491

@kcrisman

This comment has been minimized.

@kcrisman kcrisman changed the title Bring two more plot3d files to 100% coverage Bring three more plot3d files to 100% coverage Feb 10, 2012
@kcrisman
Copy link
Member Author

comment:2

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 function-call syntax. Given that this was deprecated in consolidate in plotting all extraction of variables, ranges, and fast_float setup #7008 (and let me tell you, figuring that out was nontrivial!) and Sage-4.1.2, from October 2009, I am going to remove this.

@kcrisman
Copy link
Member Author

comment:3

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.

@kcrisman
Copy link
Member Author

Based on 5.0.beta3

@kcrisman
Copy link
Member Author

comment:4

Attachment: trac_12491.patch.gz

It seems to pass all tests, and I looked at all the examples I added. Coverage is good. Needs review!

@kcrisman
Copy link
Member Author

Author: Karl-Dieter Crisman

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 12, 2012

comment:5

Apply trac_12491.patch, trac_12491-review.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.

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 12, 2012

Reviewer: David Loeffler

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 12, 2012

Attachment: trac_12491-review.patch.gz

Apply over previous patch

@loefflerd
Copy link
Mannequin

loefflerd mannequin commented Mar 12, 2012

comment:6

Apply trac_12491.patch, trac_12491-review.patch

(for the patchbot, which spotted a trailing whitespace character in my previous patch)

@kcrisman
Copy link
Member Author

comment:7

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.

@jhpalmieri
Copy link
Member

comment:8

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:

diff --git a/sage/plot/plot3d/plot3d.py b/sage/plot/plot3d/plot3d.py
--- a/sage/plot/plot3d/plot3d.py
+++ b/sage/plot/plot3d/plot3d.py
@@ -108,8 +108,9 @@ class _Coordinates(object):
 
             sage: from sage.plot.plot3d.plot3d import _ArbitraryCoordinates as 
             sage: x,y,z=var('x,y,z')
-            sage: arb((x+z,y*z,z), z, (x,y))
+            sage: c=arb((x+z,y*z,z), z, (x,y)); c
             Arbitrary Coordinates coordinate transform (z in terms of x, y)
+            sage: TestSuite(c).run()
         """
         import inspect
         all_vars=inspect.getargspec(self.transform).args[1:]

then I get this failure:

sage -t  "devel/sage-main/sage/plot/plot3d/plot3d.py"       
**********************************************************************
File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/sage-5.0.beta7-gcc/devel/sage-main/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/sage-5.0.beta7-gcc/local/lib/python/site-packages/sage/misc/sage_unittest.py", line 275, in run
        test_method(tester = tester)
      File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/sage-5.0.beta7-gcc/local/lib/python/site-packages/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/sage-5.0.beta7-gcc/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/sage-5.0.beta7-gcc/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).

@kcrisman
Copy link
Member Author

comment:9

Thanks, John. Definitely another ticket - this pickling stuff is always tricky.

@kcrisman
Copy link
Member Author

Changed reviewer from David Loeffler to David Loeffler, John Palmieri

@jdemeyer
Copy link

Merged: sage-5.0.beta9

@fchapoton

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants