Opened 14 years ago

Closed 14 years ago

#6006 closed enhancement (fixed)

[with patch, positive review] Bring plot/point.py to 100% coverage

Reported by: kcrisman Owned by: kcrisman
Priority: minor Milestone: sage-4.0
Component: documentation Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Bring plot/point.py to 100% coverage.

Attachments (3)

trac_6006.patch (8.9 KB) - added by kcrisman 14 years ago.
trac_6006-fix.patch (1.1 KB) - added by kcrisman 14 years ago.
trac_6006-reviewer.patch (627 bytes) - added by mvngu 14 years ago.
reviewer patch

Download all attachments as: .zip

Change History (12)

Changed 14 years ago by kcrisman

Attachment: trac_6006.patch added

comment:1 Changed 14 years ago by kcrisman

Owner: changed from tba to kcrisman
Status: newassigned
Summary: Bring plot/point.py to 100% coverage[with patch, needs review] Bring plot/point.py to 100% coverage

This patch also actually implements the height option for the plot3d method of the Point class that the old doctest was actually using through a 3D graphics object.

comment:2 Changed 14 years ago by kcrisman

I should also point out that I could not find a way to test for _allowed_options for the keywords in Point.plot3d, so I put something about that in the TODO but also pointed it out in the documentation, lest one confuse pointsize and size (et al.).

comment:4 Changed 14 years ago by mvngu

Summary: [with patch, needs review] Bring plot/point.py to 100% coverage[with patch, needs work] Bring plot/point.py to 100% coverage

On line 43, the constructor for Point(GraphicPrimitive_xydata) is

43	    def __init__(self, xdata, ydata, options)

Currently with Sphinx, any docstring of __init__ will not be shown in the documentation for Point(GraphicPrimitive_xydata). The situation is the same for every class. So one issue to fix is to copy the examples in __init__(self, xdata, ydata, options) and paste them within the docstring for Point(GraphicPrimitive_xydata) after line 32. Another issue is, the constructor arguments xdata, ydata, options on line 43 are not documented in the docstring for Point(GraphicPrimitive_xydata). Constructor arguments must be documented to explain what they are, and how to properly use them.

comment:5 Changed 14 years ago by kcrisman

I was not aware of this. I would be happy to improve this, and copying examples is easy. However, could you point me to a file with a correctly documented set of constructor arguments? I am uncertain how to proceed with that, particularly because I modeled my own documentation after other examples in plot/; the xdata and ydata, for instance, come from the function getxydata or something like that, which is called when a point (as opposed to Point) is made. Also, what should be in the docstring for init, if not what is currently there? Thanks for any ideas or model files to look at!

comment:6 in reply to:  4 Changed 14 years ago by mabshoff

Replying to mvngu: }

Currently with Sphinx, any docstring of __init__ will not be shown in the documentation for Point(GraphicPrimitive_xydata). The situation is the same for every class. So one issue to fix is to copy the examples in __init__(self, xdata, ydata, options) and paste them within the docstring for Point(GraphicPrimitive_xydata) after line 32.

No, do not do this. It will be fixed in the future. This is still "needs work" due to the other issue.

Cheers,

Michael

Changed 14 years ago by kcrisman

Attachment: trac_6006-fix.patch added

comment:7 Changed 14 years ago by kcrisman

Summary: [with patch, needs work] Bring plot/point.py to 100% coverage[with patch, needs review] Bring plot/point.py to 100% coverage

Changed 14 years ago by mvngu

Attachment: trac_6006-reviewer.patch added

reviewer patch

comment:8 Changed 14 years ago by mvngu

Milestone: sage-4.0.1sage-4.0
Summary: [with patch, needs review] Bring plot/point.py to 100% coverage[with patch, positive review] Bring plot/point.py to 100% coverage

Positive review! Apply patches in the following order:

  1. trac_6006.patch
  2. trac_6006-fix.patch
  3. trac_6006-reviewer.patch -- this fixes two trivial typos introduced in trac_6006.patch

comment:9 Changed 14 years ago by mabshoff

Resolution: fixed
Status: assignedclosed

Merged all three patches in Sage 4.0.alpha0.

Cheers,

Michael

Note: See TracTickets for help on using tickets.