#20343 closed enhancement (fixed)
Add file `sage/misc/latex_standalone.py` and class `TikzPicture`
Reported by:  slabbe  Owned by:  

Priority:  major  Milestone:  sage9.6 
Component:  misc  Keywords:  
Cc:  dkrenn, jipilab, slelievre, chapoton  Merged in:  
Authors:  Sébastien Labbé  Reviewers:  JeanPhilippe Labbé, Xavier Caruso, Vincent Delecroix, Matthias Koeppe 
Report Upstream:  N/A  Work issues:  
Branch:  5c9b41c (Commits, GitHub, GitLab)  Commit:  
Dependencies:  #32650, #33005  Stopgaps: 
Description (last modified by )
The goal of this ticket is to import the TikzPicture
module from the optional SageMath package slabbe into SageMath. This module has been created in 2016 and has evolved to a stable state and it is mature enough to go into SageMath now.
Here is a demo inside a Jupyter notebook (notice that the import line was since changed to from sage.misc.latex_standalone import TikzPicture
):
More examples:
sage: from sage.misc.latex_standalone import TikzPicture sage: V = [[1,0,1],[1,0,0],[1,1,0],[0,0,1],[0,1,0],[1,0,0],[0,1,1],[0,0,1],[0,1,0]] sage: P = Polyhedron(vertices=V).polar() sage: s = P.projection().tikz([674,108,731],112) sage: t = TikzPicture(s) sage: t.pdf() # opens the pdf in a viewer '/home/slabbe/.sage/temp/miami/5032/tikz_gvem9vu8.pdf' sage: t.pdf('image.pdf') # when a filename is provided, it doesn't open in a viewer '/home/slabbe/image.pdf'
It may create the image into png format (translation pdf > png made with convert from imagemagick) or svg format (translation pdf > svg made with pdf2svg):
sage: path_to_file = t.png() sage: path_to_file = t.svg() sage: path_to_file = t.tex()
In a Jupyter notebook, this shows the picture below the cell (using rich repr):
sage: t
In the terminal, this shows the header and tail of the tikzpicture code:
sage: t \documentclass[tikz]{standalone} \usepackage{amsmath} \begin{document} \begin{tikzpicture}% [x={(0.249656cm, 0.577639cm)}, y={(0.777700cm, 0.358578cm)}, z={(0.576936cm, 0.733318cm)}, scale=1.000000,  91 lines not printed (5169 characters in total). Use print to see the full content.  \node[vertex] at (0.00000, 1.00000, 0.00000) {}; \node[vertex] at (0.50000, 0.50000, 0.50000) {}; %% %% \end{tikzpicture} \end{document}
Use print
to see the full content:
sage: print(t)
A second example shows that it is faster than view and better (avoids to crop the vertices):
sage: g = graphs.PetersenGraph() sage: %time _ = TikzPicture(latex(g), standalone_options=["border=4mm"], usepackage=['tkzgraph']).pdf() CPU times: user 3.52 ms, sys: 12.7 ms, total: 16.2 ms Wall time: 2.24 s
compared to using view (which crops the vertices and creates two pages pdf on mac):
sage: %time view(g, tightpage=True) CPU times: user 126 ms, sys: 85 ms, total: 211 ms Wall time: 6.06 s
Also, if dot2tex and graphviz available, one may directly construct a tikzpicture from a sagemath graph:
sage: t = TikzPicture.from_graph(g, prog='dot') # optional: dot2tex
The tikzpicture.py
was developped within optional package slabbe
since 2015. Its interface should be stable enough to go into sage now.
Meanwhile any user can use this tikzpicture module by installing the package:
sage pip install slabbe
and
sage: from slabbe import TikzPicture
An example of usage of this module can be found at this url:
associated to the preprint
published in Journal of Modern Dynamics at
Attachments (1)
Change History (172)
comment:1 Changed 6 years ago by
 Description modified (diff)
comment:2 Changed 6 years ago by
 Branch set to u/slabbe/20343
 Commit set to 476b574c4198115e6e98e3396ed32f4b95077c35
 Status changed from new to needs_review
comment:3 Changed 6 years ago by
 Description modified (diff)
comment:4 Changed 6 years ago by
 Summary changed from Adding sage/misc/tikz_picture.py to Adding sage/misc/tikzpicture.py
comment:5 followup: ↓ 6 Changed 6 years ago by
 Cc dkrenn added
A comment:
t = TikzPicture(latex(transducers.GrayCode())) t.pdf()
does not work as the preamble is not set correctly. However, there is
from sage.combinat.finite_state_machine import setup_latex_preamble setup_latex_preamble()
(as well as the corresponding function in sage.graphs.graph_latex
) to set this correctly, but it seems to be ignored by TikzPicture
.
In contrast,
view(transducers.GrayCode())
works, but I have no idea if setup_latex_preamble
is called (not needed to be done manually) or if tikz is included anyways.
comment:6 in reply to: ↑ 5 ; followup: ↓ 7 Changed 6 years ago by
Replying to dkrenn:
A comment:
t = TikzPicture(latex(transducers.GrayCode())) t.pdf()does not work as the preamble is not set correctly.
As it is now, the user must set the libraries. This should work:
t = TikzPicture(latex(transducers.GrayCode()), tikzlibraries=['automata']) t.pdf()
I am still not sure about the proper way to choose the arguments for the __init__
function and their especially their default values. Should the arguments be empty by default? equal to the current sage latex preamble? or full of a big load of libraries?
comment:7 in reply to: ↑ 6 Changed 6 years ago by
Replying to slabbe:
Replying to dkrenn:
A comment:
t = TikzPicture(latex(transducers.GrayCode())) t.pdf()does not work as the preamble is not set correctly.
As it is now, the user must set the libraries. This should work:
t = TikzPicture(latex(transducers.GrayCode()), tikzlibraries=['automata']) t.pdf()
Works, thanks.
I am still not sure about the proper way to choose the arguments for the
__init__
function and their especially their default values. Should the arguments be empty by default? equal to the current sage latex preamble? or full of a big load of libraries?
Without much thinking, I vote for: The second, equal to the current sage latex preamble.
comment:8 Changed 6 years ago by
 Commit changed from 476b574c4198115e6e98e3396ed32f4b95077c35 to 4ae7f63de1517e4c483e500757989b6efde5b785
comment:9 Changed 6 years ago by
 Description modified (diff)
comment:10 Changed 6 years ago by
 Branch u/slabbe/20343 deleted
 Commit 4ae7f63de1517e4c483e500757989b6efde5b785 deleted
 Description modified (diff)
My module has evolved in my package slabbe, therefore the branch here is not up to date anymore. Also, I intend to make it more general during the next year allowing any content not only tikzpicture code. Therefore, I deleted the branch posted here.
comment:11 Changed 6 years ago by
 Description modified (diff)
 Status changed from needs_review to needs_work
Changing status to needs work.
comment:12 Changed 5 years ago by
 Cc jipilab added
 Reviewers set to JeanPhilippe Labbé
As a followup ticket to make use of this is #22506.
comment:13 Changed 16 months ago by
 Branch set to u/caruso/tikzpicture
comment:14 Changed 16 months ago by
 Commit set to 88c048c2a33618220a586f708273e41fd803f6bc
I'll be happy to revive this ticket.
New commits:
88c048c  import tikz_picture.py from slabbe gitlab repository

comment:15 Changed 16 months ago by
 Commit changed from 88c048c2a33618220a586f708273e41fd803f6bc to 2917f4b60238afcb4f097f3a256f734141aabe02
Branch pushed to git repo; I updated commit sha1. New commits:
2917f4b  doctests pass

comment:16 Changed 16 months ago by
 Commit changed from 2917f4b60238afcb4f097f3a256f734141aabe02 to 8cfc83431a25394be8b0dbc890d0dc0f72f54f56
Branch pushed to git repo; I updated commit sha1. New commits:
8cfc834  typos

comment:17 Changed 16 months ago by
I'm not convinced by all the methods from_dot_string
, from_graph
, etc. in the class TikzPicture
. I would say that this code should instead be in the classes implementing the object we want to display. E.g. it could be a method _tikz_
, just as _latex_
, and TikzPicture
just calls it.
What's your opinion?
comment:18 Changed 16 months ago by
There is no class for dot strings
. Graphs already have a _latex_
method which returns a tikz picture string. It is what is used with for instance view(G)
. The from_graph
class method really is using dot2tex
+ graphviz
which is not the same layout sometimes. It is true that maybe adding a _tikz_
would avoid to replace the current behavior of _latex_
. But then, having two different behaviors will be non pedagogical (how to know there is another one). Also, changing the default behavior will lead to problems...
I think it will be easier to add the new module as is and later on, in other tickets, add such _tikz_
methods in other parts of sage where it makes sense.
comment:19 Changed 16 months ago by
 Status changed from needs_work to needs_review
OK. Let's keep it for another ticket.
comment:20 Changed 16 months ago by
 Status changed from needs_review to positive_review
comment:21 Changed 16 months ago by
 Milestone changed from sage7.2 to sage9.4
 Reviewers changed from JeanPhilippe Labbé to JeanPhilippe Labbé, Xavier Caruso
comment:22 Changed 16 months ago by
please no "from future import ..."
Returns should be Return
comment:23 Changed 16 months ago by
 Status changed from positive_review to needs_work
Also if pdflatex is not installed on the machine, some doctests fail. I don't know if this should be fixed but I think so because otherwise patchbots complain.
comment:24 Changed 16 months ago by
 Branch changed from u/caruso/tikzpicture to u/slabbe/20343
 Commit changed from 8cfc83431a25394be8b0dbc890d0dc0f72f54f56 to 665db0b0dd19254278d3cd74d96423a617781223
New commits:
665db0b  20343: removed from __future__ imports

comment:25 Changed 16 months ago by
 Commit changed from 665db0b0dd19254278d3cd74d96423a617781223 to 0022eb2e8333597e2f4004e6b3a2d9363713df88
Branch pushed to git repo; I updated commit sha1. New commits:
0022eb2  20343: Returns > Return

comment:26 Changed 16 months ago by
 Commit changed from 0022eb2e8333597e2f4004e6b3a2d9363713df88 to 03aa1bccb5640263e1d23867ee742b1ce9ac3ae0
Branch pushed to git repo; I updated commit sha1. New commits:
03aa1bc  20343: adding few optional tags

comment:27 Changed 16 months ago by
 Status changed from needs_work to needs_review
I added a bunch of #optional latex
and #optional imagemagick
tags. That should fix most of the issues.
I also added one # optional pdf2svg
for which I am not sure about. I don't know if the doctest framework know about this tag. I change to needs review to check that on the patchbot.
comment:28 Changed 16 months ago by
Why do you make it part of Sage and not an independent Python module? These could easily be declared as optional/standard packages afterwards.
comment:29 followup: ↓ 31 Changed 16 months ago by
Yes, I agree with that option too. Lately, I was thinking more about this option.
But, the module also depends on many things that are in Sage which would make this module quite unusable outside of sage as it is currently. For example:
To check if Graphviz is there:
from sage.features.graphviz import Graphviz
To check if pdflatex, convert and other programs are available:
from sage.misc.latex import have_pdflatex, have_convert, have_program
To load the latex macros from sage:
from sage.misc.latex import _Latex_prefs from sage.misc.latex_macros import sage_latex_macros
To create a temporary file inside the sage folder of temporary files:
from sage.misc.temporary_file import tmp_filename
To open viewers:
from sage.misc.viewer import browser from sage.misc.viewer import pdf_viewer from sage.misc.viewer import png_viewer
To check whether we are in the JupyterNotebook or not:
from sage.repl.rich_output.backend_ipython import BackendIPythonNotebook from sage.repl.rich_output.buffer import OutputBuffer
comment:30 Changed 16 months ago by
 Commit changed from 03aa1bccb5640263e1d23867ee742b1ce9ac3ae0 to 7d590342121285b6fea577b0defc65aece99258e
Branch pushed to git repo; I updated commit sha1. New commits:
7d59034  20343: adding few optional latex tags

comment:31 in reply to: ↑ 29 Changed 14 months ago by
Replying to slabbe:
Yes, I agree with that option too. Lately, I was thinking more about this option.
The main advantage with that option is that the package can change versions independently of sage.
But, the module also depends on many things that are in Sage which would make this module quite unusable outside of sage as it is currently. For example:
This would be a Python module that depends on sage. There is nothing bad with that situation. And this is not the only one.
comment:32 Changed 14 months ago by
I am thinking about what to do now, either work on this ticket or create a new package. If I create a sagemathtikzpicture package that imports many parts of sagemath, I will be responsible for fixing the package each time sagemath changes something that I import. On the other hand, if that modules goes into sagemath, then, it will become the responsability of the person which changes the dependencies to also fix the tikzpicture module. I agree with the idea of modularization, but for the above reason, I would prefer to make it go into sage. What do you think?
comment:33 Changed 13 months ago by
I prefer to have this directly in sage since I think that there are many other places in sage where it would be great to have a tikz output.
comment:34 Changed 13 months ago by
Because this module does not depend on any external libraries, there is nothing gained in the direction of modularization by making this a separate distribution package.
But perhaps you can find a better place for it than sage.misc? How about sage.typeset
or sage.plot
?
comment:35 Changed 13 months ago by
 Commit changed from 7d590342121285b6fea577b0defc65aece99258e to 9f9f5ec90bd47bae4ffe1a3e838f1cad229619a2
Branch pushed to git repo; I updated commit sha1. New commits:
d84b6a3  Merge branch 'u/slabbe/20343' of trac.sagemath.org:sage into 20343

ee56d32  adding optional tag latex to one doctest

2692685  20343:avoid use of doctest continuation since it is not one

2acfe61  20343: misc/tikz_picture.py > plot/tikzpicture.py

388a5f8  from misc.tikz_picture > from plot.tikzpicture

b04ea0d  updated authors and copyright

9f9f5ec  20343: redoing recent changes by Vincent Delecroix in slabbe package (check_call > run); improved header documentation

comment:36 Changed 13 months ago by
 Description modified (diff)
 Summary changed from Adding sage/misc/tikzpicture.py to Adding sage/plot/tikzpicture.py
comment:37 Changed 13 months ago by
 Description modified (diff)
comment:38 Changed 13 months ago by
 Description modified (diff)
comment:39 Changed 13 months ago by
 Description modified (diff)
comment:40 Changed 13 months ago by
 Description modified (diff)
comment:41 Changed 13 months ago by
I think we are close to it. The doctests work on my machine:
sage t optional=sage,optional,external long showskipped tikzpicture.py
gives
Using optional=4ti2,ccache,cryptominisat,debian,dot2tex,e_antic,external,fricas,glucose,latte_int,lidia,normaliz,notedown,pandoc_attributes,pip,pycosat,pynormaliz,rst2ipynb,sage,sage_numerical_backends_coin,sage_spkg External software to be detected: cplex,ffmpeg,graphviz,gurobi,imagemagick,internet,latex,macaulay2,magma,maple,mathematica,matlab,octave,pandoc,rubiks,scilab Doctesting 1 file. sage t long randomseed=0 tikzpicture.py 19 not tested tests not run 1 pdf2svg test not run 0 tests not run because we ran out of time [161 tests, 15.43 s]  All tests passed!  Total time for all tests: 15.6 seconds cpu time: 8.2 seconds cumulative wall time: 15.4 seconds External software detected for doctesting: imagemagick,latex
TODO:
 Let's see what the patchbot says.
 The doctest tagged with
pdf2svg
is not recognized by theoptional=external
. Maybe something needs to be done on the detection of external packages.  One issue is that the current
_rich_repr_
always uses png instead of svg in the Jupyter Notebook even if I define'svg'
before'png'
in theprefer_vector
tuple. I think that issue can be dealt in a later ticket.
That's it! Needs review!
comment:42 Changed 13 months ago by
 Cc slelievre added
This adhoc system package advice
+ if not have_program('pdf2svg'): + raise RuntimeError("pdf2svg does not seem to be installed. " + "Install it for example with ``brew install pdf2svg``" + " or ``aptget install pdf2svg``.") +
should be replaced by a Feature
and an spkg pdf2svg
with distros/
information from
https://repology.org/project/pdf2svg/versions
comment:43 Changed 13 months ago by
Ok, I will add a Feature for pdf2svg.
Also, patchbot says this:
src/sage/plot/tikzpicture.py:98:1 'subprocess.CalledProcessError' imported but unused found 1 pyflakes errors in the modified files
Another TODO is to allow to get some verbosity useful when compilation of the picture breaks. I am writting it here not to forget.
I will work on that soon or later.
comment:44 Changed 13 months ago by
 Status changed from needs_review to needs_work
comment:45 Changed 12 months ago by
 Milestone changed from sage9.4 to sage9.5
comment:46 Changed 9 months ago by
 Dependencies set to #32650
comment:47 Changed 9 months ago by
 Commit changed from 9f9f5ec90bd47bae4ffe1a3e838f1cad229619a2 to 80151c25e7aa040973b6f6268f49ddc3b521cfd7
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
a96e94d  32650: Creating feature pdf2svg

870d461  32650: Creating features latex, xelatex, pdflatex, lualatex

385569e  32650: adding latex, xelatex, pdflatex, lualatex, pdf2svg, dvipng to doctest/external.py

f3291d8  32650: using features instead of have_program in misc/latex.py

d455993  32650: using features in plot/animate.py

cf02094  32650: using features in plot/graphics.py and plot/multigraphics.py

28f1f4a  32650: deprecating have_* functions in misc/latex.py in favor of features

5d9414d  32650: .. note > .. NOTE

6d21ec7  Merge branch #20343 onto #32650

80151c2  20343: using features instead of have_program, have_pdflatex, have_convert

comment:48 Changed 9 months ago by
comment:49 Changed 9 months ago by
 Commit changed from 80151c25e7aa040973b6f6268f49ddc3b521cfd7 to 997b8f1a58afe1b8eae04a030ec5b769fa8463b7
Branch pushed to git repo; I updated commit sha1. New commits:
997b8f1  20343: print the log when pdflatex fails compile the tikz image

comment:50 followup: ↓ 61 Changed 9 months ago by
 Status changed from needs_work to needs_review
With the recent commit and subsequent work done in #32650, the remainings TODOs are:
 improve rich repr to use svg not always png
and as I suggested before, this one could be dealt in another ticket as it is still functional with png's in the Jupyter notebook.
Needs review!
comment:51 Changed 9 months ago by
 Description modified (diff)
comment:52 Changed 9 months ago by
 Description modified (diff)
comment:53 Changed 9 months ago by
 Description modified (diff)
comment:54 followup: ↓ 58 Changed 9 months ago by
In a similar direction as comment:17, I think that the from_graph...
and from_poset
class methods should rather be changed to methods of Graph
and Poset
.
comment:55 Changed 9 months ago by
Also https://pypi.org/project/tikzplotlib/ looks interesting
comment:56 Changed 8 months ago by
 Commit changed from 997b8f1a58afe1b8eae04a030ec5b769fa8463b7 to 6a085cfdf1c930e4322a43d9d77356c2128ed913
comment:57 Changed 8 months ago by
I added two commits to do few improvements.
comment:58 in reply to: ↑ 54 ; followup: ↓ 64 Changed 8 months ago by
Replying to mkoeppe:
In a similar direction as comment:17, I think that the
from_graph...
andfrom_poset
class methods should rather be changed to methods ofGraph
andPoset
.
If you want to go in that direction, we need to have a design discussion before spending time changing the graph code. As you know,
sage: g = graphs.PetersenGraph() sage: hasattr(g, '_latex_') True
which returns an object of type:
sage: type(latex(g)) <class 'sage.misc.latex.LatexExpr'>
This latex expression starts with \begin{tikzpicture}
and ends with \end{tikzpicture}
which also depends on \usepackage{tkzgraph}
:
sage: latex(g) \begin{tikzpicture} \definecolor{cv0}{rgb}{0.0,0.0,0.0} \definecolor{cfv0}{rgb}{1.0,1.0,1.0} ... \Edge[lw=0.1cm,style={color=cv6v9,},](v6)(v9) \Edge[lw=0.1cm,style={color=cv7v9,},](v7)(v9) % \end{tikzpicture}
Notice that this output can be provided to TikzPicture
with no problem with t = TikzPicture(latex(g), usepackage=['tkzgraph'])
and t.pdf()
works etc.
I don't think here we want to replace the output of g._latex_
by an object of type TikzPicture
since a such change would break a lot of code.
An option is to create a new method def tikz(self)
instead. I can do something like that. It could return the TikzPicture(latex(g), usepackage=['tkzgraph'])
by default and if one asks, it could use the graphviz + dot2tex output instead. But, I was thinking of doing this in another ticket as a follow up, as this will open discussions that will just make longer before this ticket can go in.
Or maybe you would rather like to remove the from_graph
and the from_poset
methods to avoid to have to maintain them in the future? Well, currently, the from_graph
also allows to merge the multiple edges which is something that would not go in the tikz
method of a graph. Also, keeping the from_graph
would make the passage from this module to sage easier for me because I have been using this method over and over and over in my research code.
comment:59 Changed 8 months ago by
I created ticket #33002 for the addition of a method to return an object of type TikzPicture
in graph, posets and polyhedron code.
comment:60 Changed 8 months ago by
 Commit changed from 6a085cfdf1c930e4322a43d9d77356c2128ed913 to d6d473a1400059b385104ed4220097fde906e82f
Branch pushed to git repo; I updated commit sha1. New commits:
d6d473a  20343: fixing a bug, png output was not working

comment:61 in reply to: ↑ 50 Changed 8 months ago by
Replying to slabbe:
With the recent commit and subsequent work done in #32650, the remainings TODOs are:
 improve rich repr to use svg not always png
and as I suggested before, this one could be dealt in another ticket as it is still functional with png's in the Jupyter notebook.
Also, I want to say that I believe that the sage.repl
code is broken with respect to showing svg files.
# Jupyter cell 1 g = graphs.PetersenGraph() from sage.plot.tikzpicture import TikzPicture t = TikzPicture(latex(g), usepackage=['tkzgraph']) t # works: it shows a png
# Jupyter cell 2 from sage.repl.rich_output import get_display_manager dm = get_display_manager() assert dm.preferences.graphics is None dm.preferences.graphics = 'raster' t # works: it shows a png
# Jupyter cell 3 dm.preferences.graphics = 'vector' t # broken: it does not show anything
The following shows that the Jupyter notebook is able to handle a svg correctly:
# Jupyter cell 4 t.svg('a.svg') from IPython.core.display import SVG SVG(filename='a.svg')
I suggest to fix the issue with the showing of svg in another ticket.
comment:62 Changed 8 months ago by
 Commit changed from d6d473a1400059b385104ed4220097fde906e82f to 2da56a8cb87d3e2de789bbfd08375baee36009fd
Branch pushed to git repo; I updated commit sha1. New commits:
2da56a8  20343: using pdftocairo instead of pdf2svg

comment:63 Changed 8 months ago by
 Dependencies changed from #32650 to #32650, #33005
comment:64 in reply to: ↑ 58 Changed 8 months ago by
Replying to slabbe:
Replying to mkoeppe:
In a similar direction as comment:17, I think that the
from_graph...
andfrom_poset
class methods should rather be changed to methods ofGraph
andPoset
.
I don't think here we want to replace the output of
g._latex_
by an object of typeTikzPicture
since a such change would break a lot of code.
I agree
An option is to create a new method
def tikz(self)
instead. I can do something like that.
Yes, this is what I meant
comment:65 Changed 8 months ago by
 Milestone changed from sage9.5 to sage9.6
comment:66 Changed 7 months ago by
 Commit changed from 2da56a8cb87d3e2de789bbfd08375baee36009fd to 90817776abb4c1e1ed21ac6af983b337330d4b2c
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
d4ca003  from misc.tikz_picture > from plot.tikzpicture

42011d6  updated authors and copyright

6894a5b  20343: redoing recent changes by Vincent Delecroix in slabbe package (check_call > run); improved header documentation

2c07925  20343: using features instead of have_program, have_pdflatex, have_convert

83b9173  20343: print the log when pdflatex fails compile the tikz image

b26bc3f  20343: few small improvements

ae7b9bf  20343: improving first example shown in documentation

9f91d22  20343: fixing a bug, png output was not working

f39e07a  20343: using pdftocairo instead of pdf2svg

9081777  20343:fixing 2 doctests + wrong trailing colons

comment:67 Changed 7 months ago by
There was a syntax error in the file due to the previous commit. I fixed that.
I also rebased the branch on top of 9.5.beta9.
comment:68 Changed 7 months ago by
 Commit changed from 90817776abb4c1e1ed21ac6af983b337330d4b2c to a78e9ce673d3df77ca008af4c6e73490ff3d7ce7
comment:69 Changed 7 months ago by
I confirm it works on my side:
$ sage t tikzpicture.py [...] Doctesting 1 file. sage t randomseed=33963620693510224516081799230454604818 tikzpicture.py [158 tests, 4.59 s]  All tests passed!  Features detected for doctesting:
and
$ sage t showskipped long optional=sage,optional,external tikzpicture.py [...] Doctesting 1 file. sage t long randomseed=204572532264437349038334999704478431314 tikzpicture.py 24 not tested tests not run 2 pdftocairo tests not run 0 tests not run because we ran out of time [188 tests, 15.97 s]  All tests passed!  Features detected for doctesting: imagemagick,latex,pdf2svg
Changed 7 months ago by
comment:70 Changed 7 months ago by
 Description modified (diff)
Updating the description: adding a screenshot of the behavior in Jupyter, hoping it motivates someone to review the ticket.
comment:71 Changed 7 months ago by
 Description modified (diff)
comment:72 Changed 7 months ago by
 Description modified (diff)
comment:73 Changed 7 months ago by
 Description modified (diff)
comment:74 Changed 7 months ago by
Why this default usepackage=['amsmath']
?
comment:75 Changed 7 months ago by
Moreover, you should never have a list as a default argument which is plugged as an attribute
sage: class A(): ....: def __init__(self, a=[]): ....: self.a = a sage: x = A() sage: x.a.append(2) sage: y = A() sage: y.a [2]
comment:76 Changed 7 months ago by
 Commit changed from a78e9ce673d3df77ca008af4c6e73490ff3d7ce7 to fc47455b050b1a21244e6ab00ef3b88de8bf4d67
comment:77 followup: ↓ 78 Changed 7 months ago by
You are right. I also avoided the usage of shell=True
for security reasons, see https://docs.python.org/3/library/subprocess.html#securityconsiderations
Doing so, the cmd
needs to be a list and not a string. This is better also because it allows filenames to contain spaces: "Providing a sequence of arguments is generally preferred, as it allows the module to take care of any required escaping and quoting of arguments (e.g. to permit spaces in file names)", see https://docs.python.org/3/library/subprocess.html#frequentlyusedarguments
comment:78 in reply to: ↑ 77 ; followup: ↓ 80 Changed 7 months ago by
Replying to slabbe:
[...] This is better also because it allows filenames to contain spaces [...]
If 'm y p i c t u r e.pdf'
is an explicitely supported filename, it would make sense to mention it and test it. If it is an obscure feature, nevermind.
comment:79 Changed 7 months ago by
 Commit changed from fc47455b050b1a21244e6ab00ef3b88de8bf4d67 to ab3ec0299f903dc6cd5d25bf4cba0eb2aa4e47a1
Branch pushed to git repo; I updated commit sha1. New commits:
ab3ec02  20343: add doctest showing spaces in filename

comment:80 in reply to: ↑ 78 Changed 7 months ago by
Replying to vdelecroix:
If
'm y p i c t u r e.pdf'
is an explicitely supported filename, it would make sense to mention it and test it. If it is an obscure feature, nevermind.
Yes, why not. I added a doctest confirming it works.
I also added a missing long doctest tag.
Needs review!
comment:81 Changed 7 months ago by
 Milestone changed from sage9.6 to sage9.5
There is a time window right now for this ticket to get into sage9.5 maybe within the first rc.
The patchbots are green. I will be available in the next days to fix quickly any remaining issues.
comment:82 Changed 7 months ago by
 Reviewers changed from JeanPhilippe Labbé, Xavier Caruso to JeanPhilippe Labbé, Xavier Caruso, Vincent Delecroix
 Status changed from needs_review to positive_review
comment:83 Changed 7 months ago by
Thank you!! I was holding by breathe since 6 years!
I will add a nice section in the release notes if it makes it in 9.5.
comment:84 Changed 6 months ago by
 Milestone changed from sage9.5 to sage9.6
comment:85 Changed 6 months ago by
 Status changed from positive_review to needs_work
********************************************************************** File "src/sage/plot/tikzpicture.py", line 73, in sage.plot.tikzpicture Failed example: _ = t.png(view=False) # long time (2s) # optional imagemagick Exception raised: Traceback (most recent call last): File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/doctest/forker.py", line 694, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/doctest/forker.py", line 1088, in compile_and_execute exec(compiled, globs) File "<doctest sage.plot.tikzpicture[14]>", line 1, in <module> _ = t.png(view=False) # long time (2s) # optional imagemagick File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/plot/tikzpicture.py", line 505, in png _filename_pdf = self.pdf(filename=None, view=False) File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/plot/tikzpicture.py", line 443, in pdf result.check_returncode() File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/subprocess.py", line 460, in check_returncode raise CalledProcessError(self.returncode, self.args, self.stdout, subprocess.CalledProcessError: Command '['lualatex', 'interaction=nonstopmode', 'tikz_544oxd7o.tex']' returned nonzero exit status 1. ********************************************************************** File "src/sage/plot/tikzpicture.py", line 494, in sage.plot.tikzpicture.StandaloneTex.png Failed example: path_to_file = t.png(filename) # long time (2s) # optional imagemagick Exception raised: Traceback (most recent call last): File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/doctest/forker.py", line 694, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/doctest/forker.py", line 1088, in compile_and_execute exec(compiled, globs) File "<doctest sage.plot.tikzpicture.StandaloneTex.png[7]>", line 1, in <module> path_to_file = t.png(filename) # long time (2s) # optional imagemagick File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/plot/tikzpicture.py", line 505, in png _filename_pdf = self.pdf(filename=None, view=False) File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/plot/tikzpicture.py", line 443, in pdf result.check_returncode() File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/subprocess.py", line 460, in check_returncode raise CalledProcessError(self.returncode, self.args, self.stdout, subprocess.CalledProcessError: Command '['lualatex', 'interaction=nonstopmode', 'tikz_ikxvujw2.tex']' returned nonzero exit status 1. ********************************************************************** File "src/sage/plot/tikzpicture.py", line 495, in sage.plot.tikzpicture.StandaloneTex.png Failed example: path_to_file[4:] # long time (fast) # optional imagemagick Exception raised: Traceback (most recent call last): File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/doctest/forker.py", line 694, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/doctest/forker.py", line 1088, in compile_and_execute exec(compiled, globs) File "<doctest sage.plot.tikzpicture.StandaloneTex.png[8]>", line 1, in <module> path_to_file[Integer(4):] # long time (fast) # optional imagemagick NameError: name 'path_to_file' is not defined ********************************************************************** File "src/sage/plot/tikzpicture.py", line 579, in sage.plot.tikzpicture.StandaloneTex.svg Failed example: path_to_file = t.svg(filename, program='pdftocairo') # long time (2s) # optional pdftocairo Exception raised: Traceback (most recent call last): File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/doctest/forker.py", line 694, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/doctest/forker.py", line 1088, in compile_and_execute exec(compiled, globs) File "<doctest sage.plot.tikzpicture.StandaloneTex.svg[7]>", line 1, in <module> path_to_file = t.svg(filename, program='pdftocairo') # long time (2s) # optional pdftocairo File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/plot/tikzpicture.py", line 588, in svg _filename_pdf = self.pdf(filename=None, view=False) File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/plot/tikzpicture.py", line 443, in pdf result.check_returncode() File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/subprocess.py", line 460, in check_returncode raise CalledProcessError(self.returncode, self.args, self.stdout, subprocess.CalledProcessError: Command '['lualatex', 'interaction=nonstopmode', 'tikz_o2ho82xz.tex']' returned nonzero exit status 1. ********************************************************************** File "src/sage/plot/tikzpicture.py", line 580, in sage.plot.tikzpicture.StandaloneTex.svg Failed example: path_to_file[4:] # long time (fast) # optional pdftocairo Exception raised: Traceback (most recent call last): File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/doctest/forker.py", line 694, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/release/Sage/local/var/lib/sage/venvpython3.9.9/lib/python3.9/sitepackages/sage/doctest/forker.py", line 1088, in compile_and_execute exec(compiled, globs) File "<doctest sage.plot.tikzpicture.StandaloneTex.svg[8]>", line 1, in <module> path_to_file[Integer(4):] # long time (fast) # optional pdftocairo NameError: name 'path_to_file' is not defined **********************************************************************
comment:86 Changed 6 months ago by
Indeed, the failures show that when latex is not installed, there are missing optional latex
tags. Sorry Volker.
comment:87 Changed 6 months ago by
 Commit changed from ab3ec0299f903dc6cd5d25bf4cba0eb2aa4e47a1 to 4b5bd661c86a07eab7ebef50bafd4d4822d4730f
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
c8c9cb1  20343: improving first example shown in documentation

3aae6c0  20343: fixing a bug, png output was not working

65ab05b  20343: using pdftocairo instead of pdf2svg

cd8e604  20343:fixing 2 doctests + wrong trailing colons

0790c85  20343: change default from pdf2svg to pdftocairo

aef6bd4  20343: provide log if an error occurs in method png

d4695c0  20343: change default value of usepackage to None

d057886  20343: use shell=False instead

a933774  20343: add doctest showing spaces in filename

4b5bd66  20343: adding missing optional latex tag

comment:88 Changed 6 months ago by
Added the missing latex tag. Also rebased on 9.6.beta1.
comment:89 Changed 6 months ago by
 Status changed from needs_work to needs_review
comment:90 followup: ↓ 93 Changed 6 months ago by
Why not using else
here
+ if program == 'pdflatex': + pdflatex().require() + elif program == 'lualatex': + lualatex().require() + elif program not in ['pdflatex','lualatex']: + raise ValueError("program(={}) should be pdflatex or lualatex".format(program))
comment:91 followup: ↓ 94 Changed 6 months ago by
Why do you prefix filename_png
, filename_pdf
, etc with an additional underscore, ie _filename_png
, _filename_pdf
?
comment:92 Changed 6 months ago by
 Commit changed from 4b5bd661c86a07eab7ebef50bafd4d4822d4730f to f1e4728d00efd130a0688ac79e616bffbae0c539
Branch pushed to git repo; I updated commit sha1. New commits:
f1e4728  20343: using else instead of elif

comment:93 in reply to: ↑ 90 Changed 6 months ago by
Replying to vdelecroix:
Why not using
else
here
You are right. I don't know why I did not use else. I added a commit to fix that.
comment:94 in reply to: ↑ 91 ; followup: ↓ 95 Changed 6 months ago by
Replying to vdelecroix:
Why do you prefix
filename_png
,filename_pdf
, etc with an additional underscore, ie_filename_png
,_filename_pdf
?
The reason is to distinguish the variable filename
(input given from the user) from the variable _filename
(name of a temporary file created by the method). See the code of the method svg
where both filename
and _filename
are used. For consistency, the same principle is used in all methods.
Needs review!
comment:95 in reply to: ↑ 94 ; followup: ↓ 97 Changed 6 months ago by
Replying to slabbe:
Replying to vdelecroix:
Why do you prefix
filename_png
,filename_pdf
, etc with an additional underscore, ie_filename_png
,_filename_pdf
?The reason is to distinguish the variable
filename
(input given from the user) from the variable_filename
(name of a temporary file created by the method). See the code of the methodsvg
where bothfilename
and_filename
are used. For consistency, the same principle is used in all methods.Needs review!
Instead of the very unconventional _filename
variable name you would better use temporary_filename
, temporary_filename_tex
, temporary_filename_pdf
, etc.
comment:96 Changed 6 months ago by
 Commit changed from f1e4728d00efd130a0688ac79e616bffbae0c539 to 2f4771798e2c4219794d36890babbf65a44f2cfd
Branch pushed to git repo; I updated commit sha1. New commits:
2f47717  20343: rename _filename* > temp_filename*

comment:97 in reply to: ↑ 95 Changed 6 months ago by
Replying to vdelecroix:
Instead of the very unconventional
_filename
variable name you would better usetemporary_filename
,temporary_filename_tex
,temporary_filename_pdf
, etc.
Ok, I renamed it as s/_filename/temp_filename/g
.
Needs review!
comment:98 Changed 6 months ago by
Instead of from_graph
, from_poset
, ... wouldn't it make more sense to have Graph.tikz()
, Poset.tikz()
, etc? In the former design the TikzPicture
code would end up with a ton of methods dealing with various specific objects.
comment:99 Changed 6 months ago by
Or alternatively: __tikz__()
if you want to hide it.
comment:100 Changed 6 months ago by
See comment:17 :)
comment:101 followup: ↓ 102 Changed 6 months ago by
I agree with Xavier ! The current branch proposes to make TikzPicture.from_XXX
the default conversion function. If the design is wrong, better to change it now than in a hypothetical future.
comment:102 in reply to: ↑ 101 Changed 6 months ago by
Replying to vdelecroix:
I agree with Xavier ! The current branch proposes to make
TikzPicture.from_XXX
the default conversion function. If the design is wrong, better to change it now than in a hypothetical future.
See also comment:54 from mkoeppe who suggested the same thing.
Then, please read followup comment:58, and answer to my answer in comment:58.
comment:103 followups: ↓ 104 ↓ 105 Changed 6 months ago by
Ideally, TikzPicture
should inherit from LatexExpr
so that returning a TikzPicture
from a _latex_
method would not be a breaking change.
For the purpose of merging this ticket quickly, the simplest way out is to move the from_XXX
as private standalone functions def _from_XXX
and stating explicitely that these are just demo functions that will be removed in the future.
comment:104 in reply to: ↑ 103 Changed 6 months ago by
Replying to vdelecroix:
Ideally,
TikzPicture
should inherit fromLatexExpr
so that returning aTikzPicture
from a_latex_
method would not be a breaking change.
Well, LatexExpr
already inherits from the Python class str
which makes it full of methods that, as a user, I don't want to see in the list of methods seen with tabcompletion on an object of type TikzPicture
.
For the purpose of merging this ticket quickly, the simplest way out is to move the
from_XXX
as private standalone functionsdef _from_XXX
and stating explicitely that these are just demo functions that will be removed in the future.
Ok, let me think about that again, maybe this Thursday at Bordeaux Sage Thursdays, we can discuss this and converge to choices on how this is done for graphs (tkzberge
vs graphviz + dot2tex) and for polytopes (the actual tikz()
method vs a new tikz()
or __tikz__()
method).
comment:105 in reply to: ↑ 103 ; followup: ↓ 106 Changed 6 months ago by
Replying to vdelecroix:
For the purpose of merging this ticket quickly, the simplest way out is to move the
from_XXX
as private standalone functionsdef _from_XXX
and stating explicitely that these are just demo functions that will be removed in the future.
There are four such methods right now:
def from_dot_string(cls, dotdata, prog='dot'): def from_graph(cls, graph, merge_multiedges=True, def from_graph_with_pos(cls, graph, scale=1, merge_multiedges=True, def from_poset(cls, poset, **kwds):
As you suggest, I can change those taking a graph or a poset sage object into a private method:
def from_dot_string(cls, dotdata, prog='dot'): def _from_graph(cls, graph, merge_multiedges=True, def _from_graph_with_pos(cls, graph, scale=1, merge_multiedges=True, def _from_poset(cls, poset, **kwds):
But, there is no sage object for a dot string. So, do I keep from_dot_string
as it is or do you want me to make it private as well?
comment:106 in reply to: ↑ 105 ; followup: ↓ 108 Changed 6 months ago by
Replying to slabbe:
Replying to vdelecroix:
For the purpose of merging this ticket quickly, the simplest way out is to move the
from_XXX
as private standalone functionsdef _from_XXX
and stating explicitely that these are just demo functions that will be removed in the future.There are four such methods right now:
def from_dot_string(cls, dotdata, prog='dot'): def from_graph(cls, graph, merge_multiedges=True, def from_graph_with_pos(cls, graph, scale=1, merge_multiedges=True, def from_poset(cls, poset, **kwds):As you suggest, I can change those taking a graph or a poset sage object into a private method:
def from_dot_string(cls, dotdata, prog='dot'): def _from_graph(cls, graph, merge_multiedges=True, def _from_graph_with_pos(cls, graph, scale=1, merge_multiedges=True, def _from_poset(cls, poset, **kwds):But, there is no sage object for a dot string. So, do I keep
from_dot_string
as it is or do you want me to make it private as well?
It should be fine to keep from_dot_string
.
The only reason to make the three others private is to avoid that sage users rely on them. They will be changed without notice.
comment:107 Changed 5 months ago by
 Commit changed from 2f4771798e2c4219794d36890babbf65a44f2cfd to 793fbdeba581155a5f21c2a07ec81f1de6a5e396
Branch pushed to git repo; I updated commit sha1. New commits:
793fbde  20343: from_graph, from_poset > _from_graph, _from_poset

comment:108 in reply to: ↑ 106 Changed 5 months ago by
Replying to vdelecroix:
It should be fine to keep
from_dot_string
.The only reason to make the three others private is to avoid that sage users rely on them. They will be changed without notice.
Ok, I did that. Needs review.
comment:109 Changed 5 months ago by
It would be better not to advertize them in the module docstring
 If dot2tex Sage optional package and graphviz are installed, then the following
 one liner works allowing to create a nice tikzpicture from a graph with
 vertices and edges placed according to graphviz::

 sage: t = TikzPicture._from_graph(g) # optional: dot2tex # long time (3s)

And you can add a sentence explaining the transducer example that comes after.
comment:110 Changed 5 months ago by
I would also add a sentence somewhere in the module docstring about the fact that tikz is a latex package (called pgftikz
by the way) and a reference to the page https://www.ctan.org/pkg/pgf. You sort of assume that everybody knows tikz which is not a reasonable assumption.
comment:111 Changed 5 months ago by
 Commit changed from 793fbdeba581155a5f21c2a07ec81f1de6a5e396 to 06c8fda7e1b115e1ef35a0ab605ebe24f719efc2
Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:
2539054  20343:fixing 2 doctests + wrong trailing colons

1ed6a0c  20343: change default from pdf2svg to pdftocairo

6111d2a  20343: provide log if an error occurs in method png

21fe0d3  20343: change default value of usepackage to None

a424ed5  20343: use shell=False instead

f9dcd5e  20343: add doctest showing spaces in filename

ad234af  20343: adding missing optional latex tag

32f5efa  20343: using else instead of elif

dde1186  20343: rename _filename* > temp_filename*

06c8fda  20343: from_graph, from_poset > _from_graph, _from_poset

comment:112 Changed 5 months ago by
I rebased the branch on 9.6.beta2. No new commit for now.
comment:113 Changed 5 months ago by
 Commit changed from 06c8fda7e1b115e1ef35a0ab605ebe24f719efc2 to 83b367cb93f1ba8a974884669939250f72d2d4f2
Branch pushed to git repo; I updated commit sha1. New commits:
83b367c  Removed _from_graph from module doc

comment:114 Changed 5 months ago by
The following reference will not work :mod:`sage.misc.latex.py`
(you need to remove the .py
).
comment:115 Changed 5 months ago by
 Commit changed from 83b367cb93f1ba8a974884669939250f72d2d4f2 to bb314c215cd5797ff979e792882c0e5dcc5086a5
Branch pushed to git repo; I updated commit sha1. New commits:
bb314c2  20343: adding info about pgf/tikz + removed acknowledgement since the code has evolved so much that it has nothing to do with sage.misc.latex anymore

comment:116 Changed 5 months ago by
 Commit changed from bb314c215cd5797ff979e792882c0e5dcc5086a5 to 55649f6924baab86e35045f9a1e8876c385e2fb9
Branch pushed to git repo; I updated commit sha1. New commits:
55649f6  20343: removed a TODO

comment:117 Changed 5 months ago by
Needs review!
comment:118 Changed 5 months ago by
pycodestyle is broken as it does not detect the prefix r
in front of the string...
src/sage/plot/tikzpicture.py:258:34: W605 invalid escape sequence '\d' src/sage/plot/tikzpicture.py:259:34: W605 invalid escape sequence '\e'
comment:119 Changed 5 months ago by
 Status changed from needs_review to needs_work
Your module does not seem to be included in the reference manual. You have to add an entry to src/doc/en/reference/misc/index.rst
.
comment:120 Changed 5 months ago by
 Commit changed from 55649f6924baab86e35045f9a1e8876c385e2fb9 to e6d0a972bd3ec403f2ae1f48043072e5a1eca849
comment:122 followup: ↓ 123 Changed 5 months ago by
I did not notice before but why did you put your module in sage/plot
rather than sage/misc
(where all the current latex code is)?
comment:123 in reply to: ↑ 122 ; followup: ↓ 124 Changed 5 months ago by
Replying to vdelecroix:
I did not notice before but why did you put your module in
sage/plot
rather thansage/misc
(where all the current latex code is)?
This was suggested in comment:34 by mkoeppe, to which I agree. The goal of the module is about drawing stuff, so it is natural to put it in sage/plot
.
It is not true that all latex code is in misc. You also have graph_latex.py
file in sage/graphs
.
Also, to my opinion, sage/misc/latex.py
is not soooo wellwritten (I repeat, this is an opinion). It is a module on which it is tough to build on top / reuse. You may want to look at the code which implements view
to get an example. Who really wants to use this? I think the future will be more active if the features present in that module are rethought and rewritten else where and eventually it gets just replaced. The deprecation of the have_latex
and friends methods done in #32650 is one of the many steps towards that. The creation of a module to deal with tikzpicture is another step towards that.
comment:124 in reply to: ↑ 123 Changed 5 months ago by
Replying to slabbe:
This was suggested in comment:34 by mkoeppe, to which I agree.
Not sure if I still agree with myself ;)
comment:125 Changed 5 months ago by
It might be true that sage.misc.latex
is a mess. But given that
tikzpicture.py
shares some functionalities withsage.misc.latex
(including code duplication)sage.misc.latex
is widely used you claim that
StandaloneTex
/TikzPicture
is part of the future ofsage.misc.latex
I would like to
 have an idea of what is the future of
sage.misc.latex
(you said that a step forward but there is no destination)  have the tools that will replace
sage.misc.latex
close to it in the sage source code or at the very least a mention of them in its documentation
What I want to avoid is that this ticket only adds a module n+1
to sagemath which does a small variation of module 1
, module 2
and module n
independently of all of them.
comment:126 Changed 5 months ago by
 Status changed from needs_review to needs_work
Ok, let me do more changes (next week).
comment:127 Changed 5 months ago by
 Commit changed from e6d0a972bd3ec403f2ae1f48043072e5a1eca849 to ba46d5125b59988ca8690f55c62a45408374597f
Branch pushed to git repo; I updated commit sha1. New commits:
92e8443  20343: mv plot/tikzpicture.py > misc/latex_standalone.py

8a09032  20343: mv plot/tikzpicture.py > misc/latex_standalone.py (in documentation index.rst)

58e7acd  20343: plot.tikzpicture > misc.latex_standalone

d15ce29  20343: StandaloneTex > Standalone

ba46d51  20343: improved module doc + enhance the Standalone module

comment:128 Changed 5 months ago by
 Commit changed from ba46d5125b59988ca8690f55c62a45408374597f to fb5c4f56ccdbb2e4631a73cfd046b6b1dff4e5f4
Branch pushed to git repo; I updated commit sha1. New commits:
fb5c4f5  20343: fix pdftocairo feature import

comment:129 Changed 5 months ago by
 Commit changed from fb5c4f56ccdbb2e4631a73cfd046b6b1dff4e5f4 to d4f4f426e4eb82df1ea2f96305ee14248b3d136b
Branch pushed to git repo; I updated commit sha1. New commits:
d4f4f42  20343: improved class and module doc

comment:130 Changed 5 months ago by
 Status changed from needs_work to needs_review
Gave more importance to the Standalone
class. Updated and improved the documentation.
Needs review!
comment:131 Changed 5 months ago by
 Branch changed from u/slabbe/20343 to u/caruso/20343
comment:132 Changed 5 months ago by
 Branch changed from u/caruso/20343 to u/slabbe/20343
 Commit changed from d4f4f426e4eb82df1ea2f96305ee14248b3d136b to 4e4b4a4a89c405e245b77bcb405b5ec9ab713627
comment:133 Changed 5 months ago by
Needs review!
comment:134 Changed 5 months ago by
+Loading few latex packages::
> "a" few
comment:135 Changed 5 months ago by
 Cc chapoton added
+#***************************************************************************** +# Copyright (C) 20152022 Sébastien Labbé <slabqc@gmail.com> +# +# Distributed under the terms of the GNU General Public License version 2 (GPLv2) +# +# The full text of the GPLv2 is available at: +# +# http://www.gnu.org/licenses/
This is not our standard license header; are you sure you want to mark it as "v2 only"? Other files say "Distributed under the terms of the GNU General Public License (GPL); either version 2 of the License, or (at your option) any later version."
comment:136 Changed 5 months ago by
+Adding a border in the options avoids croping the vertices of a graph::
> cropping
comment:137 followup: ↓ 144 Changed 5 months ago by
+ sage: t = TikzPicture(s, standalone_config=["border=4mm"], usepackage=['tkzgraph']) # optional latex + sage: _ = t.pdf(view=False) # long time (2s) # optional latex
This should also be marked # optional  latex_package_tkz_graph
(occurs several times)
comment:138 followup: ↓ 145 Changed 5 months ago by
+ sage: dotdata = G.graphviz_string() + sage: tikz = TikzPicture.from_dot_string(dotdata) # optional dot2tex # long time (3s)
this should probably also be marked as # optional  graphviz
comment:139 followups: ↓ 146 ↓ 148 Changed 5 months ago by
Also, could you please mark the doctests using graphs as # optional  sage.graphs
and the doctests using posets as # optional  sage.combinat
?
comment:140 Changed 5 months ago by
Overall looking great!
comment:141 Changed 5 months ago by
Thanks for your review! I will improve the code accordingly.
comment:142 Changed 5 months ago by
 Commit changed from 4e4b4a4a89c405e245b77bcb405b5ec9ab713627 to 3ed5d51a47ae8c3dd3c8090f33e73422b63d393f
comment:143 Changed 5 months ago by
It remains to fix the doctests tags. I will do this later.
comment:144 in reply to: ↑ 137 Changed 5 months ago by
Replying to mkoeppe:
+ sage: t = TikzPicture(s, standalone_config=["border=4mm"], usepackage=['tkzgraph']) # optional latex + sage: _ = t.pdf(view=False) # long time (2s) # optional latexThis should also be marked
# optional  latex_package_tkz_graph
(occurs several times)
Only the line t.pdf()
really needs the latex_package_tkz_graph
optional tag. So I added this tag where necessary. Creating a Python string containing '\usetikzlibrary{tkzgraph}'
by itself does not depend on the latex package. So, I did not added the optional tag everywhere.
comment:145 in reply to: ↑ 138 Changed 5 months ago by
Replying to mkoeppe:
+ sage: dotdata = G.graphviz_string() + sage: tikz = TikzPicture.from_dot_string(dotdata) # optional dot2tex # long time (3s)this should probably also be marked as
# optional  graphviz
At some point in time, it was changed that the installation of dot2tex
fails if graphviz
is not installed. But, whatever, maybe it is better to add graphviz
optional tag as well, which I did (commits to be pushed soonafter).
comment:146 in reply to: ↑ 139 Changed 5 months ago by
Replying to mkoeppe:
Also, could you please mark the doctests using graphs as
# optional  sage.graphs
and the doctests using posets as# optional  sage.combinat
?
Ok, done.
comment:147 Changed 5 months ago by
 Commit changed from 3ed5d51a47ae8c3dd3c8090f33e73422b63d393f to 36c3ebb5c2fa064b8c0f7e262ba2c3c06e9a9216
comment:148 in reply to: ↑ 139 Changed 5 months ago by
Replying to mkoeppe:
Also, could you please mark the doctests using graphs as
# optional  sage.graphs
and the doctests using posets as# optional  sage.combinat
?
Also I could make the doctests less dependent on graphs stuff.
comment:149 Changed 5 months ago by
 Commit changed from 36c3ebb5c2fa064b8c0f7e262ba2c3c06e9a9216 to bc9186f91cb6a8f0d55d0e9eb1d7f18fe831dc1a
Branch pushed to git repo; I updated commit sha1. New commits:
936e82d  20343: updating example in docstrings of Standalone class to use Standalone class instead of inherited class TikzPicture

e90292c  20343: adding methods to append to the lists of options

bc9186f  20343: add a deprecation message for include_header > content_only

comment:150 Changed 5 months ago by
Needs review!
comment:151 followup: ↓ 152 Changed 5 months ago by
 Reviewers changed from JeanPhilippe Labbé, Xavier Caruso, Vincent Delecroix to JeanPhilippe Labbé, Xavier Caruso, Vincent Delecroix, Matthias Koeppe
comment:152 in reply to: ↑ 151 Changed 5 months ago by
Replying to mkoeppe:
These lines and what depends on them should be marked
# optional  sage.symbolic
(for #32432, #32601)+ sage: f(x) = 1 / x + sage: g(x) = 1 / (x + 1)Other than that, from my side it's positive review.
Alternatively, use lambda functions instead of symbolic ones:
 sage: f(x) = 1 / x  sage: g(x) = 1 / (x + 1) + sage: f = lambda x: 1 / x + sage: g = lambda x: 1 / (x + 1)
comment:153 Changed 5 months ago by
No, they wouldn't show up well in the plots.
comment:154 followup: ↓ 157 Changed 5 months ago by
 Status changed from needs_review to positive_review
Great! I think it is time to give a positive review to this ticket!
comment:155 Changed 5 months ago by
 Commit changed from bc9186f91cb6a8f0d55d0e9eb1d7f18fe831dc1a to 5c9b41c96e4213242fee4af98b934914fb4d2360
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
5c9b41c  20343: adding doctest tag sage.symbolic

comment:156 Changed 5 months ago by
 Summary changed from Adding sage/plot/tikzpicture.py to Adding sage/misc/latex_standalone.py + class TikzPicture
comment:157 in reply to: ↑ 154 Changed 5 months ago by
Replying to caruso:
Great! I think it is time to give a positive review to this ticket!
I added the doctests tags sage.symbolic
to answer the latest comment. Needs review again. Sorry.
I also updated the title of the ticket.
comment:158 Changed 5 months ago by
 Description modified (diff)
comment:159 Changed 5 months ago by
 Description modified (diff)
comment:160 Changed 5 months ago by
Patchbot is green. We can change the status to positive review.
But, maybe before, we could check if all tests pass on a machine with no latex installed, because Volker is using such a computer to test tickets, see comment:85. Do someone has a machine with no latex? Or is there an easy way to mimic its absence?
comment:161 Changed 5 months ago by
 Description modified (diff)
 Summary changed from Adding sage/misc/latex_standalone.py + class TikzPicture to Add file `sage/misc/latex_standalone.py` and class `TikzPicture`
comment:162 Changed 5 months ago by
There must be a way to use tox for that...
comment:163 Changed 5 months ago by
I found a computer with no latex and no imagemagick:
$ pdflatex v La commande « pdflatex » n'a pas été trouvée, mais peut être installée avec : sudo apt install texlivelatexbase
$ lualatex v La commande « lualatex » n'a pas été trouvée, mais peut être installée avec : sudo apt install texlivelatexbase
$ convert v La commande « convert » n'a pas été trouvée, mais peut être installée avec : sudo apt install graphicsmagickimagemagickcompat # version 1.4+really1.3.351, or sudo apt install imagemagick6.q16 # version 8:6.9.10.23+dfsg2.1ubuntu11.4 sudo apt install imagemagick6.q16hdri # version 8:6.9.10.23+dfsg2.1ubuntu11.4
on which I installed sage:
$ ./sage version SageMath version 9.6.beta5, Release Date: 20220312
on which I merged this ticket, I confirm the doctests work:
./sage t src/sage/misc/latex_standalone.py no stored timings available Running doctests with ID 2022032411083282df3068. Git branch: 20343 Using optional=debian,pip,sage,sage_spkg Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_jones_numfield,database_knotinfo,dvipng,graphviz,imagemagick,jupymake,kenzo,latte_int,lrslib,mcqd,meataxe,nauty,palp,pandoc,pdf2svg,pdftocairo,plantri,polytopes_db,polytopes_db_4d,pynormaliz,python_igraph,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.groups,sage.plot,sage.rings.number_field,sage.rings.padics,sage.rings.real_double,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,sphinx,tdlib Doctesting 1 file. sage t randomseed=224621707555152601217192014946498015018 src/sage/misc/latex_standalone.py [216 tests, 0.29 s]  All tests passed!  Total time for all tests: 0.3 seconds cpu time: 0.3 seconds cumulative wall time: 0.3 seconds Features detected for doctesting: graphviz,sage.combinat,sage.graphs,sage.symbolic Pytest is not installed, skip checking tests that rely on it.
$ ./sage t optional=sage,optional,external long src/sage/misc/latex_standalone.py too few successful tests, not using stored timings Running doctests with ID 2022032411085406354475. Git branch: 20343 Using optional=debian,external,pip,sage,sage_spkg Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,cplex,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_jones_numfield,database_knotinfo,dvipng,ffmpeg,graphviz,gurobi,imagemagick,internet,jupymake,kenzo,latex,latex_package_tkz_graph,latte_int,lrslib,lualatex,macaulay2,magma,maple,mathematica,matlab,mcqd,meataxe,nauty,octave,palp,pandoc,pdf2svg,pdflatex,pdftocairo,plantri,polytopes_db,polytopes_db_4d,pynormaliz,python_igraph,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.groups,sage.plot,sage.rings.number_field,sage.rings.padics,sage.rings.real_double,sage.symbolic,sage_numerical_backends_coin,sagemath_doc_html,scilab,sphinx,tdlib,xelatex Doctesting 1 file. sage t long randomseed=318237908393715923968305484155979250546 src/sage/misc/latex_standalone.py [216 tests, 0.29 s]  All tests passed!  Total time for all tests: 0.4 seconds cpu time: 0.3 seconds cumulative wall time: 0.3 seconds Features detected for doctesting: graphviz,pdftocairo,sage.combinat,sage.graphs,sage.symbolic Pytest is not installed, skip checking tests that rely on it.
comment:164 Changed 5 months ago by
 Status changed from needs_review to positive_review
Nice! I give back a positive review to this ticket.
comment:165 Changed 5 months ago by
Super!
comment:166 Changed 4 months ago by
 Branch changed from u/slabbe/20343 to 5c9b41c96e4213242fee4af98b934914fb4d2360
 Resolution set to fixed
 Status changed from positive_review to closed
comment:167 Changed 4 months ago by
 Commit 5c9b41c96e4213242fee4af98b934914fb4d2360 deleted
this has introduced wrong indentation
src/sage/misc/latex_standalone.py:688:12: E111 indentation is not a multiple of four src/sage/misc/latex_standalone.py:689:16: E111 indentation is not a multiple of four src/sage/misc/latex_standalone.py:690:12: E111 indentation is not a multiple of four src/sage/misc/latex_standalone.py:691:16: E111 indentation is not a multiple of four
comment:168 Changed 4 months ago by
so please swiftly review #33635
comment:169 followup: ↓ 171 Changed 2 months ago by
Could this possibly be leading to the bad LaTeX that's causing the problem with view(crystals.Tableaux("A3",shape=[2,1]))
— see https://groups.google.com/g/sagesupport/c/1u4mOsz6jY/m/PAM1C40AwAJ?
comment:170 Changed 2 months ago by
In case this is relevant to the view(crystals....)
failure, that is now #33947.
comment:171 in reply to: ↑ 169 Changed 2 months ago by
Replying to jhpalmieri:
Could this possibly be leading to the bad LaTeX that's causing the problem with
view(crystals.Tableaux("A3",shape=[2,1]))
— see https://groups.google.com/g/sagesupport/c/1u4mOsz6jY/m/PAM1C40AwAJ?
No. The problem reported on sagesupport is with respect to Sage 9.3. The TikzPicture
code went into sage in 9.6. Also, for now, no other file is using the new TikzPicture
module.
New commits:
20343: Adding tikzpicture.py in sage/misc