#7889: revolution plot
-----------------------------------------+----------------------------------
Reporter: olazo | Owner: olazo
Type: enhancement | Status: needs_review
Priority: minor | Milestone: sage-4.3.3
Component: graphics | Keywords: revolution,plot
Author: Oscar Gerardo Lazo Arjona | Upstream: N/A
Reviewer: Karl-Dieter Crisman | Merged:
Work_issues: |
-----------------------------------------+----------------------------------
Changes (by olazo):
* status: needs_work => needs_review
Comment:
I just uploaded another patch. It rewrites a lot of things.
Replying to [comment:6 kcrisman]:
> Trivial typos:
Taken care of.
> I am uncomfortable with
> {{{
> def findvar(expr):#find the dependent variable of the curve
> try:
> vart=curve.args()[0]
> except:
> vart=var('t')
> return vart
> }}}
> because even if there isn't a default choice, t could conceivably mean
something else, but var() injects it into the global namespace (I think?).
> I think that using isinstance is generally preferred, also. The timing
isn't really that important here, but I think it is more "Pythonic" and:
> {{{
> sage: %timeit str(type(curve)) == "<type 'tuple'>"
> 625 loops, best of 3: 869 ns per loop
> sage: %timeit isinstance(curve,tuple)
> 625 loops, best of 3: 264 ns per loop
> }}}
>
All of that has been taken care of
> Just curious - why the xz-plane and not the xy-plane for the default
location of the axis of rotation? Obviously it doesn't "really" matter,
but at least in the US most texts start rotating from there, so if this is
intended for pedagogical purposes it could be confusing. I don't know
what Mathematica does, though, nor whether this is standard for industrial
or non-US uses of revolution plots.
Actually, the xy plane is where the axis used to be by default (I left
that unchanged). But I've added the posibility to choose to which
coordinate axis the revolution axis will be parallel.
> Next,
> {{{
> from sage.symbolic.constants import pi
> from sage.functions.other import sqrt
> from sage.functions.trig import sin
> from sage.functions.trig import cos
> }}}
> It looks like you probably need sin and cos to be symbolic, though I'm
not sure whether you need to import them. But I think your use of pi and
sqrt would be sufficient to import from the Python/C library math, since
they are only being used to compute actual numbers, not symbolics,
correct? Like this
> {{{
> from math import pi, sqrt
> }}}
I tried that, but it produced errors
> Please document phirange with examples - they are so cool! You should
probably also allow a tuple like (-pi,pi/2) in phirange, since there is
absolutely no ambiguity (check len(phirange)==2 or ==3 to do this) and
then people don't have to create a new variable
> {{{
> sage: var('phi')
> sage: revolution_plot3d...(phi,-pi,pi/2)...
> }}}
> which would be quite annoying, as I just discovered when trying to get a
phirange other than 0 to 2*pi.
phirange now takes a 2-tuple. I had quite a hard time making that work
propper.
> You may also want 'curve' to be able to be a Vector object (symbolic),
but I am not quite sure whether/how that is supported in such cases. Jason
will know, since he has done stuff with this.
It should be enough to use revolution_plot3d(tuple(your_vector),...).
I'll upload some screenshots once my connection recovers a reasonable
speed.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/7889#comment:8>
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.
