#12083: Add a TikZ-tex output method for 2d and 3d polytopes
---------------------------+------------------------------------------------
Reporter: jipilab | Owner: mhampton
Type: enhancement | Status: needs_work
Priority: minor | Milestone: sage-5.0
Component: geometry | Keywords: TikZ, polytope
Work_issues: | Upstream: N/A
Reviewer: | Author:
Merged: | Dependencies:
---------------------------+------------------------------------------------
Changes (by slabbe):
* status: needs_review => needs_work
Comment:
1. Testing the file {{{polyhedra.py}}} creates junk in the working
directory.
{{{
cd sage-4.8/devel/sage-slabbe/sage/geometry
sage -t polyhedra.py
ls *.tex
polytope-tikz.tex polytope-tikz2.tex polytope-tikz3.tex
}}}
Saving files in your test is not necessary. I suggest you do the test like
this instead :
{{{
#!python
sage: P1 = polytopes.small_rhombicuboctahedron()
sage: t = P1.tikz([1,3,5], 175, scale=4)
sage: type(t)
<type 'str'>
sage: print '\n'.join(t.splitlines()[:4])
\begin{tikzpicture}%
[x={(-0.939161cm, 0.244762cm)},
y={(0.097442cm, -0.482887cm)},
z={(0.329367cm, 0.840780cm)},
}}}
If you want to keep those line in the doc (a good idea a believe), I
suggest you mark them as untested :
{{{
sage: open('polytope-tikz2.tex', 'w').write(Image2) # not tested
}}}
2. You import {{{n}}} but you don't use it. (If you really needed it, I
would have suggest to import the equivalent alias {{{numerical_approx}}}
instead because the variable {{{n}}} is often used as an integer in the
code...) But since you don't use it, just remove the import.
{{{
#!diff
from subprocess import Popen, PIPE
from sage.misc.all import tmp_filename
-from sage.misc.functional import norm
+from sage.misc.functional import norm, n
from sage.misc.package import is_package_installed
}}}
3. Testing the file {{{polyhedra.py}}} is not significantly longer than
before : Great!
sage-4.8:
{{{
$ sage -t polyhedra.py
sage -t "devel/sage-slabbe/sage/geometry/polyhedra.py"
[58.0 s]
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 58.0 seconds
}}}
sage-4.8 + your patch:
{{{
$ sage -t polyhedra.py
sage -t "devel/sage-slabbe/sage/geometry/polyhedra.py"
[58.6 s]
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 58.6 seconds
}}}
4. You import VectorSpace but vector could do the job...
{{{
#!diff
from sage.modules.free_module_element import vector
+from sage.modules.free_module import VectorSpace
from sage.matrix.constructor import matrix, identity_matrix
}}}
In your code, you write :
{{{
V = VectorSpace(RDF,3)
view_vector = V(view)
}}}
I suggest you change this to :
{{{
view_vector = vector(RDF, view)
}}}
5. Every methods must have doctests:
{{{
$ sage -coverage polyhedra.py
----------------------------------------------------------------------
polyhedra.py
SCORE polyhedra.py: 98% (214 of 217)
Missing doctests:
* _tikz_2d(self, scale, edge_color, facet_color, opacity,
vertex_color, axis):
* _tikz_2d_in_3d(self, view, rot_angle, scale, edge_color,
facet_color, opacity, vertex_color, axis):
* _tikz_3d_in_3d(self, view, rot_angle, scale, edge_color,
facet_color, opacity, vertex_color, axis):
----------------------------------------------------------------------
}}}
6. I tested my script {{{tikz2pdf}}} on the above 3 junk tex files created
by the doctest. Pdf files were created without problem. Pictures look
great!
7. Building the documentation creates 2 warnings :
{{{
$ sage -docbuild reference html
...
/Users/slabbe/Applications/sage-4.8/local/lib/python2.6/site-
packages/sage/geometry/polyhedra.py:docstring
of sage.geometry.polyhedra.Polyhedron.tikz:26: WARNING: Inline literal
start-string without end-string.
/Users/slabbe/Applications/sage-4.8/local/lib/python2.6/site-
packages/sage/geometry/polyhedra.py:docstring
of sage.geometry.polyhedra.Polyhedron.tikz:27: WARNING: Inline literal
start-string without end-string.
...
}}}
I believe the problem comes from this :
{{{
#!diff
+ 2) Select ``Console`;
+ 3) Select the tab ``State`;
}}}
8. Change this
{{{
It read something like:
moveto 0.0 {x y z angle} Scale
}}}
to
{{{
It read something like::
moveto 0.0 {x y z angle} Scale
}}}
It will look nicer in the documentation.
9. I suggest to change the variable name {{{rot_angle}}} to {{{angle}}}.
10. Input block do not follow the coding convention.
See www.sagemath.org/doc/developer/conventions.html
I suggest you follow this example :
{{{
INPUT:
- ``x`` - integer (default: 1) the description of the
argument x goes here. If it contains multiple lines, all
the lines after the first need to be indented.
- ``y`` - integer (default: 2) the ...
OUTPUT:
integer -- the ...
}}}
So, first say what is the type. Then, say what is the default (inside
{{{``}}} quotes. Then, give the description.
For the output, just say the type and what it is. The part
{{{``tikz_pic``}}} is not necessary.
11. The description of {{{view}}} argument could be improved.
{{{
- ``view`` -- a list of length 3 representing a vector;
}}}
I know it is a vector. Is it the camera position? Is it the view angle? Is
it the axis for the rotation?
12. I think the type of what you return should be a {{{LatexExpr}}}
instead of {{{str}}}.
{{{
from sage.misc.latex import LatexExpr
}}}
Compare {{{latex(Image1)}}} to {{{latex(LatexExpr(Image1))}}}. Because,
sagetex
will call {{{latex}}} function on your output...
Bonne fête !
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12083#comment:4>
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.
