Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#12491 closed enhancement (fixed)

Bring three more plot3d files to 100% coverage

Reported by: kcrisman Owned by: jason, was
Priority: major Milestone: sage-5.0
Component: graphics Keywords:
Cc: Merged in: sage-5.0.beta9
Authors: Karl-Dieter Crisman Reviewers: David Loeffler, John Palmieri
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by chapoton)

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)

trac_12491.patch (23.1 KB) - added by kcrisman 8 years ago.
Based on 5.0.beta3
trac_12491-review.patch (6.8 KB) - added by davidloeffler 8 years ago.
Apply over previous patch

Download all attachments as: .zip

Change History (13)

comment:1 Changed 8 years ago by kcrisman

  • 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 kcrisman

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

comment:3 Changed 8 years ago by kcrisman

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.

Changed 8 years ago by kcrisman

Based on 5.0.beta3

comment:4 Changed 8 years ago by kcrisman

  • Authors set to Karl-Dieter Crisman
  • 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 davidloeffler

  • Reviewers set to David Loeffler

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.

Changed 8 years ago by davidloeffler

Apply over previous patch

comment:6 Changed 8 years ago by davidloeffler

Apply trac_12491.patch, trac_12491-review.patch

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

comment:7 follow-up: Changed 8 years ago by kcrisman

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 jhpalmieri

  • 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): 
    108108
    109109            sage: from sage.plot.plot3d.plot3d import _ArbitraryCoordinates as
    110110            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
    112112            Arbitrary Coordinates coordinate transform (z in terms of x, y)
     113            sage: TestSuite(c).run()
    113114        """
    114115        import inspect
    115116        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).

comment:9 Changed 8 years ago by kcrisman

  • 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 jdemeyer

  • Merged in set to sage-5.0.beta9
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:11 Changed 5 years ago by chapoton

  • Description modified (diff)
Note: See TracTickets for help on using tickets.