#12083 closed enhancement (fixed)
Add a TikZ-tex output method for 2d and 3d polytopes
Reported by: | jipilab | Owned by: | mhampton |
---|---|---|---|
Priority: | minor | Milestone: | sage-5.13 |
Component: | geometry | Keywords: | TikZ, polytope |
Cc: | Merged in: | sage-5.13.beta3 | |
Authors: | Jean-Philippe Labbé | Reviewers: | Sébastien Labbé, Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
I recently had to draw polytopes for my work.
Many drawing tools already exist. But none of them satisfied me.
So far, I wrote a method that takes a polytope in 3d and a view-angle (taken from Jmol) and outputs a .tex file containing a TikZ image of the polytope.
The method should also be able to deal with 2-polytopes.
The result is highly customizable: colors, edge style, vertices style, etc...
First review: I made the suggested corrections. They are present in the patch trac12083_tikz_polytope_review.patch
Test passed on version 4.7.2
Attachments (6)
Change History (30)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
I just need to adapt the code a little bit. It is already mostly written and works with 'quite' big examples: A3,B3 and H3 permutahedra.
I just need to sit and do it: it should be there in a week or so...
comment:3 Changed 10 years ago by
- Status changed from new to needs_review
Here is the patch. Please test it with your favorite 2d and 3d polytopes!
comment:4 Changed 10 years ago by
- Status changed from needs_review to needs_work
- 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 :
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
- You import
n
but you don't use it. (If you really needed it, I would have suggest to import the equivalent aliasnumerical_approx
instead because the variablen
is often used as an integer in the code...) But since you don't use it, just remove the import.
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
- 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
- You import VectorSpace? but vector could do the job...
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)
- 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): ----------------------------------------------------------------------
- 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!
- 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 :
+ 2) Select ``Console`;
+ 3) Select the tab ``State`;
- 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.
- I suggest to change the variable name
rot_angle
toangle
.
- 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.
- 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?
- I think the type of what you return should be a
LatexExpr
instead ofstr
.
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 !
comment:5 Changed 10 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
comment:6 Changed 10 years ago by
I really like this functionality and I thought I'd help by rebasing the patch to sage-5.0, but I have now run out of time. Working on this reminded me of some things I don't like about the Projection class, but I am not sure about the best way to fix them. I think projections should be more accessible from Polyhedron objects, and vice-versa. Anyway, I did complete a patch for sage-5.0 that manages to pass the original tests, but its not very pretty. Its inefficient, and relies on a parent_polyhedron method for projections that might be objectionable. But since it works, it might be helpful in creating a better version.
If no one else does, I might have time to revisit this in a week or so.
comment:7 Changed 10 years ago by
- Reviewers set to Sébastien Labbé
- Status changed from needs_review to needs_work
So - which patches have to be applied? I suspect they'll need rebasing after all Volker's refactoring.
comment:8 Changed 10 years ago by
- Status changed from needs_work to needs_review
let us try and see
apply trac12083_tikz_polytope.2.patch trac12083_tikz_polytope_review.patch
comment:9 Changed 9 years ago by
So, today I used the reviewed patch to draw a polytope using the library decorations of tikz.
I realized that I was sloppy to implement the edges; they are drawn twice. This also makes the compilation in LaTeX slower.
I will write a patch for version 5.3 that does not have repetitions (and containing the improvements in the rebased version).
comment:10 Changed 9 years ago by
So, here is the rebased version of the patch.
I solved a few problems: The vertices and edges appeared twice before (sloppy implementation); There was one vertex not drawn in the first (unholy) rebased version; The facets were badly drawn (problem with vertices naming in the code); The back edges were badly recognized.
I looked at the compiled tikz output examples and now everything seems in order.
All tests passed on 5.4.1
One thing I don't really like though is the fact that we need to pass to projections to access the tikz method. Perhaps a wrapper would do the thing?
comment:11 Changed 9 years ago by
For the bot:
apply trac12083_rebased_5.4.1.patch
comment:12 Changed 9 years ago by
There is a problem with the lines
v1 = self.coords[index1] v2 = self.coords[index2]
which appear three times, but the variables v1 and v2 are never used.
comment:13 Changed 9 years ago by
Moreover, three tests are failing on sage 5.8.beta1
comment:14 Changed 9 years ago by
- Status changed from needs_review to needs_work
This patch should probably be rebased. It would be nice to add in the ticket's description which patch are to be applied, too.
Nathann
comment:15 Changed 9 years ago by
- Milestone changed from sage-5.11 to sage-5.12
comment:16 Changed 9 years ago by
- Description modified (diff)
- Status changed from needs_work to needs_review
here is a rebased patch, passing tests and working on 5.12.beta0
apply only trac12083_rebased_5.12.patch
comment:17 Changed 9 years ago by
apply only trac12083_rebased_5.12.patch
comment:18 Changed 9 years ago by
- Reviewers changed from Sébastien Labbé to Sébastien Labbé, Volker Braun
- Status changed from needs_review to positive_review
lgtm
comment:19 Changed 9 years ago by
The PDF doc doesn't build:
! Package inputenc Error: Keyboard character used is undefined (inputenc) in inputencoding `utf8'. See the inputenc package documentation for explanation. Type H <return> for immediate help. ... l.17893 \item[{^^Hegin\{tikzpicture\}\%}] \leavevmode
To fix this, use r"""
docstrings instead of """
.
comment:20 Changed 9 years ago by
- Status changed from positive_review to needs_work
Changed 9 years ago by
comment:21 Changed 9 years ago by
I have made a corrected patch, hopefully.
apply trac12083_rebased_5.12.patch
comment:23 Changed 9 years ago by
- Merged in set to sage-5.13.beta3
- Resolution set to fixed
- Status changed from positive_review to closed
comment:24 Changed 8 years ago by
I just want to say that I could have used this a couple years ago and instead had to do lots of guess-and-check with graph plots, then convert to xfig... thank you, this will be very useful!
Do you have a ready patch already? Such functionality can be quite awesome in conjunction with SageTeX!