#9076: plot Arc of circle and ellipse
-----------------------------------+----------------------------------------
Reporter: vdelecroix | Owner: vdelecroix
Type: defect | Status: needs_work
Priority: major | Milestone: sage-4.4.4
Component: graphics | Keywords: plot, arc, ellipse, circle
Author: Vincent Delecroix | Upstream: N/A
Reviewer: Karl-Dieter Crisman | Merged:
Work_issues: |
-----------------------------------+----------------------------------------
Changes (by kcrisman):
* status: needs_review => needs_work
Comment:
I am sorry for the following laundry list. I want to reiterate that in
general this is very nice, but the hope is that this will be the best
possible! Thanks for your patience.
* I think that the current algorithm for the bounding box is not the same
as that in the worksheet, which seems to be pretty good. The ellipse part
works; I did find one example where the arc one didn't quite work:
{{{
my_arc=arc((1,-2),2,5,-pi/5,(3*pi/2,pi/4),thickness=2)
my_d=my_arc[0].get_minmax_data()
my_arc+polygon([(my_d['xmin'],my_d['ymin']),
(my_d['xmin'],my_d['ymax']),
(my_d['xmax'],my_d['ymax']),
(my_d['xmax'],my_d['ymin'])],rgbcolor='red')
}}}
{{{
my_arc=arc((0,0),2,5,-pi/5,(3*pi/2,pi/4),thickness=2)
my_xmin,my_ymin,my_xmax,my_ymax=arc_bounding_box(2,5,-pi/5,3*pi/2,pi/4)
my_arc+polygon([(my_xmin,my_ymin),
(my_xmin,my_ymax),
(my_xmax,my_ymax),
(my_xmax,my_ymin)],rgbcolor='red')
}}}
Maybe the part of the algorithm that checks whether to use the sector
endpoints is too lax?
* The worksheet is REALLY COOL, by the way; I strongly encourage you to
add more commentary, turn the graphics into interacts, and advertise it as
a great way to use Sage to demonstrate basic uses of parametric calculus!
* With respect to `fmod` - yes, this makes sense to use. (Otherwise we'd
have to document the dummy function!)
* I'd also point out that since you already imported pi from math, there
should be no need to float it (? I think?); this happens a lot in this
code.
{{{
----------------------------------------------------------------------
| Sage Version 4.4.2, Release Date: 2010-05-19 |
| Type notebook() for the GUI, and license() for information. |
----------------------------------------------------------------------
sage: from math import pi
sage: pi
3.1415926535897931
sage: float(pi)
3.1415926535897931
}}}
* On a related point, the constructor should be as minimal as possible;
why not wait to float the radii etc. until `_render_on_subplot_()` and
`get_minmax_data()`? That would also make the representation string much
easier on the eyes (and intelligible).
* Space missing in the sage/plot/plot.py reference
* My sense is that the point of having
`...@rename_keyword(color='rgbcolor')` is so that
{{{
sage: arc((0,0), 2, 1, 0, (0,pi/2), color="red")
}}}
would work, which is more natural to the user (though I may be
misunderstanding the documentation for sage.plot.misc.rename_keyword). I
realize that circle.py doesn't really take advantage of this... and really
an rgbcolor should be a 3-tuple, morally speaking... That example doesn't
have linestyle='--', incidentally.
* Center should still be 2-tuple, not 2-uple
* One may want to point out that alpha refers to transparency.
* Minh will surely point out that we wish uniformity on 2-d, 2D, 2-D, or
whatever. I think the current convention is 2-D, though some docs have 2d
or 2D. But at any rate within a file it should be consistent.
* Again, in the docs for `arc()`, the radii do NOT have to be floats.
* should there be a check for negative radii, like in the patch for
ellipse.py? `arc((1,1),1,-1,pi,(pi/2,pi))` is logically correct but maybe
would allow bugs in code of people using `arc` to slip through.
But again all of this takes nothing away from what is in general a great
contribution.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9076#comment:12>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
