Opened 6 years ago

Closed 4 months ago

# Add file sage/misc/latex_standalone.py and class TikzPicture

Reported by: Owned by: slabbe major sage-9.6 misc dkrenn, jipilab, slelievre, chapoton Sébastien Labbé Jean-Philippe Labbé, Xavier Caruso, Vincent Delecroix, Matthias Koeppe N/A 5c9b41c #32650, #33005

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=['tkz-graph']).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

### comment:1 Changed 6 years ago by slabbe

• Description modified (diff)

### comment:2 Changed 6 years ago by slabbe

• Branch set to u/slabbe/20343
• Commit set to 476b574c4198115e6e98e3396ed32f4b95077c35
• Status changed from new to needs_review

New commits:

 ​476b574 20343: Adding tikzpicture.py in sage/misc

### comment:3 Changed 6 years ago by slabbe

• Description modified (diff)

### comment:5 follow-up: ↓ 6 Changed 6 years ago by dkrenn

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 ; follow-up: ↓ 7 Changed 6 years ago by slabbe

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 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 git

• Commit changed from 476b574c4198115e6e98e3396ed32f4b95077c35 to 4ae7f63de1517e4c483e500757989b6efde5b785

Branch pushed to git repo; I updated commit sha1. New commits:

 ​7512091 20343: Adding tikzpicture.py in sage/misc ​4ae7f63 20343: improving the default argument choices for TikzPicture

### comment:9 Changed 6 years ago by slabbe

• Description modified (diff)

### comment:10 Changed 6 years ago by slabbe

• 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 slabbe

• Description modified (diff)
• Status changed from needs_review to needs_work

Changing status to needs work.

### comment:12 Changed 5 years ago by jipilab

• Reviewers set to Jean-Philippe Labbé

As a follow-up ticket to make use of this is #22506.

### comment:13 Changed 16 months ago by caruso

• Branch set to u/caruso/tikzpicture

### comment:14 Changed 16 months ago by caruso

• 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 git

• 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 git

• 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 caruso

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.

### comment:18 Changed 16 months ago by slabbe

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 caruso

• Status changed from needs_work to needs_review

OK. Let's keep it for another ticket.

### comment:20 Changed 16 months ago by caruso

• Status changed from needs_review to positive_review

### comment:21 Changed 16 months ago by caruso

• Milestone changed from sage-7.2 to sage-9.4
• Reviewers changed from Jean-Philippe Labbé to Jean-Philippe Labbé, Xavier Caruso

### comment:22 Changed 16 months ago by chapoton

please no "from future import ..."

Returns should be Return

### comment:23 Changed 16 months ago by caruso

• 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 slabbe

• 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 git

• 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 git

• 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 slabbe

• 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 vdelecroix

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 follow-up: ↓ 31 Changed 16 months ago by slabbe

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

Last edited 16 months ago by slabbe (previous) (diff)

### comment:30 Changed 16 months ago by git

• 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 vdelecroix

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 slabbe

I am thinking about what to do now, either work on this ticket or create a new package. If I create a sagemath-tikzpicture 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?

Last edited 14 months ago by slabbe (previous) (diff)

### comment:33 Changed 13 months ago by caruso

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 mkoeppe

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 git

• 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 slabbe

• Description modified (diff)

### comment:37 Changed 13 months ago by slabbe

• Description modified (diff)

### comment:38 Changed 13 months ago by slabbe

• Description modified (diff)

### comment:39 Changed 13 months ago by slabbe

• Description modified (diff)

### comment:40 Changed 13 months ago by slabbe

• Description modified (diff)

### comment:41 Changed 13 months ago by slabbe

I think we are close to it. The doctests work on my machine:

sage -t --optional=sage,optional,external --long --show-skipped 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 --random-seed=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 the --optional=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 the prefer_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 mkoeppe

+        if not have_program('pdf2svg'):
+            raise RuntimeError("pdf2svg does not seem to be installed. "
+                    "Install it for example with brew install pdf2svg"
+                    " or apt-get 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 slabbe

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 mkoeppe

• Status changed from needs_review to needs_work

### comment:45 Changed 12 months ago by mkoeppe

• Milestone changed from sage-9.4 to sage-9.5

### comment:46 Changed 9 months ago by slabbe

• Dependencies set to #32650

### comment:47 Changed 9 months ago by git

• 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 slabbe

I rebased on top of current #32650.

Current TODOS are:

• verbosity when compilation breaks
• add distro info stuff in pdf2svg (should be done in #32650)
• improve rich repr to use svg not always png

### comment:49 Changed 9 months ago by git

• 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 follow-up: ↓ 61 Changed 9 months ago by slabbe

• 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 slabbe

• Description modified (diff)

### comment:52 Changed 9 months ago by slabbe

• Description modified (diff)

### comment:53 Changed 9 months ago by slabbe

• Description modified (diff)

### comment:54 follow-up: ↓ 58 Changed 9 months ago by mkoeppe

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:56 Changed 8 months ago by git

• Commit changed from 997b8f1a58afe1b8eae04a030ec5b769fa8463b7 to 6a085cfdf1c930e4322a43d9d77356c2128ed913

Branch pushed to git repo; I updated commit sha1. New commits:

 ​f9c9492 20343: few small improvements ​6a085cf 20343: improving first example shown in documentation

### comment:57 Changed 8 months ago by slabbe

I added two commits to do few improvements.

### comment:58 in reply to: ↑ 54 ; follow-up: ↓ 64 Changed 8 months ago by slabbe

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.

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{tkz-graph}:

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=['tkz-graph']) 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=['tkz-graph']) 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 slabbe

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 git

• 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 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=['tkz-graph'])
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 git

• 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 slabbe

• Dependencies changed from #32650 to #32650, #33005

### comment:64 in reply to: ↑ 58 Changed 8 months ago by mkoeppe

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.

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.

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 mkoeppe

• Milestone changed from sage-9.5 to sage-9.6

### comment:66 Changed 7 months ago by git

• 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 slabbe

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 git

• Commit changed from 90817776abb4c1e1ed21ac6af983b337330d4b2c to a78e9ce673d3df77ca008af4c6e73490ff3d7ce7

Branch pushed to git repo; I updated commit sha1. New commits:

 ​b2388a1 20343: change default from pdf2svg to pdftocairo ​a78e9ce 20343: provide log if an error occurs in method png

### comment:69 Changed 7 months ago by slabbe

I confirm it works on my side:

$sage -t tikzpicture.py [...] Doctesting 1 file. sage -t --random-seed=33963620693510224516081799230454604818 tikzpicture.py [158 tests, 4.59 s] ---------------------------------------------------------------------- All tests passed! ---------------------------------------------------------------------- Features detected for doctesting:  and $ sage -t --show-skipped --long --optional=sage,optional,external tikzpicture.py
[...]
Doctesting 1 file.
sage -t --long --random-seed=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


### comment:70 Changed 7 months ago by slabbe

• 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 slabbe

• Description modified (diff)

### comment:72 Changed 7 months ago by slabbe

• Description modified (diff)

### comment:73 Changed 7 months ago by slabbe

• Description modified (diff)

### comment:74 Changed 7 months ago by vdelecroix

Why this default usepackage=['amsmath']?

### comment:75 Changed 7 months ago by vdelecroix

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 git

• Commit changed from a78e9ce673d3df77ca008af4c6e73490ff3d7ce7 to fc47455b050b1a21244e6ab00ef3b88de8bf4d67

Branch pushed to git repo; I updated commit sha1. New commits:

 ​a3dec92 20343: change default value of usepackage to None ​fc47455 20343: use shell=False instead

### comment:77 follow-up: ↓ 78 Changed 7 months ago by slabbe

You are right. I also avoided the usage of shell=True for security reasons, see https://docs.python.org/3/library/subprocess.html#security-considerations

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#frequently-used-arguments

Last edited 7 months ago by slabbe (previous) (diff)

### comment:78 in reply to: ↑ 77 ; follow-up: ↓ 80 Changed 7 months ago by vdelecroix

[...] 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 git

• 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 slabbe

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 slabbe

• Milestone changed from sage-9.6 to sage-9.5

There is a time window right now for this ticket to get into sage-9.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 vdelecroix

• Reviewers changed from Jean-Philippe Labbé, Xavier Caruso to Jean-Philippe Labbé, Xavier Caruso, Vincent Delecroix
• Status changed from needs_review to positive_review

### comment:83 Changed 7 months ago by slabbe

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 mkoeppe

• Milestone changed from sage-9.5 to sage-9.6

### comment:85 Changed 6 months ago by vbraun

• 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/venv-python3.9.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/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/venv-python3.9.9/lib/python3.9/site-packages/sage/plot/tikzpicture.py", line 505, in png
_filename_pdf = self.pdf(filename=None, view=False)
File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/plot/tikzpicture.py", line 443, in pdf
result.check_returncode()
File "/home/release/Sage/local/var/lib/sage/venv-python3.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 non-zero 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/venv-python3.9.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/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/venv-python3.9.9/lib/python3.9/site-packages/sage/plot/tikzpicture.py", line 505, in png
_filename_pdf = self.pdf(filename=None, view=False)
File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/plot/tikzpicture.py", line 443, in pdf
result.check_returncode()
File "/home/release/Sage/local/var/lib/sage/venv-python3.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 non-zero 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/venv-python3.9.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/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/venv-python3.9.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/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/venv-python3.9.9/lib/python3.9/site-packages/sage/plot/tikzpicture.py", line 588, in svg
_filename_pdf = self.pdf(filename=None, view=False)
File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/sage/plot/tikzpicture.py", line 443, in pdf
result.check_returncode()
File "/home/release/Sage/local/var/lib/sage/venv-python3.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 non-zero 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/venv-python3.9.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 694, in _run
self.compile_and_execute(example, compiler, test.globs)
File "/home/release/Sage/local/var/lib/sage/venv-python3.9.9/lib/python3.9/site-packages/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 slabbe

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 git

• 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 slabbe

Added the missing latex tag. Also rebased on 9.6.beta1.

### comment:89 Changed 6 months ago by slabbe

• Status changed from needs_work to needs_review

### comment:90 follow-up: ↓ 93 Changed 6 months ago by vdelecroix

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 follow-up: ↓ 94 Changed 6 months ago by vdelecroix

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 git

• 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 slabbe

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 ; follow-up: ↓ 95 Changed 6 months ago by slabbe

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 ; follow-up: ↓ 97 Changed 6 months ago by 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!

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 git

• 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 slabbe

Instead of the very unconventional _filename variable name you would better use temporary_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 vdelecroix

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 vdelecroix

Or alternatively: __tikz__() if you want to hide it.

### comment:100 Changed 6 months ago by caruso

See comment:17 :-)

### comment:101 follow-up: ↓ 102 Changed 6 months ago by 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.

### comment:102 in reply to: ↑ 101 Changed 6 months ago by slabbe

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:103 follow-ups: ↓ 104 ↓ 105 Changed 6 months ago by vdelecroix

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 slabbe

Ideally, TikzPicture should inherit from LatexExpr so that returning a TikzPicture 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 tab-completion 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 functions def _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 (tkz-berge vs graphviz + dot2tex) and for polytopes (the actual tikz() method vs a new tikz() or __tikz__() method).

### comment:105 in reply to: ↑ 103 ; follow-up: ↓ 106 Changed 6 months ago by slabbe

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.

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?

Last edited 6 months ago by slabbe (previous) (diff)

### comment:106 in reply to: ↑ 105 ; follow-up: ↓ 108 Changed 6 months ago by vdelecroix

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.

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 git

• 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 slabbe

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 vdelecroix

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 vdelecroix

I would also add a sentence somewhere in the module docstring about the fact that tikz is a latex package (called pgf-tikz 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 git

• 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 slabbe

I rebased the branch on 9.6.beta2. No new commit for now.

### comment:113 Changed 5 months ago by git

• 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 vdelecroix

The following reference will not work :mod:sage.misc.latex.py (you need to remove the .py).

Last edited 5 months ago by vdelecroix (previous) (diff)

### comment:115 Changed 5 months ago by git

• 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 git

• Commit changed from bb314c215cd5797ff979e792882c0e5dcc5086a5 to 55649f6924baab86e35045f9a1e8876c385e2fb9

Branch pushed to git repo; I updated commit sha1. New commits:

 ​55649f6 20343: removed a TODO

Needs review!

### comment:118 Changed 5 months ago by vdelecroix

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 vdelecroix

• 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 git

• Commit changed from 55649f6924baab86e35045f9a1e8876c385e2fb9 to e6d0a972bd3ec403f2ae1f48043072e5a1eca849

Branch pushed to git repo; I updated commit sha1. New commits:

 ​f8f49dc 20343: fixing few """ to r""" ​e6d0a97 20343: adding module in doc + further doc improvements

### comment:121 Changed 5 months ago by slabbe

• Status changed from needs_work to needs_review

Needs review!

### comment:122 follow-up: ↓ 123 Changed 5 months ago by vdelecroix

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 ; follow-up: ↓ 124 Changed 5 months ago by slabbe

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)?

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 well-written (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 re-thought and re-written 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.

Last edited 5 months ago by slabbe (previous) (diff)

### comment:124 in reply to: ↑ 123 Changed 5 months ago by mkoeppe

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 vdelecroix

It might be true that sage.misc.latex is a mess. But given that

1. tikzpicture.py shares some functionalities with sage.misc.latex (including code duplication)
2. sage.misc.latex is widely used
3. you claim that StandaloneTex/TikzPicture is part of the future of sage.misc.latex

I would like to

1. have an idea of what is the future of sage.misc.latex (you said that a step forward but there is no destination)
2. 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 slabbe

• Status changed from needs_review to needs_work

Ok, let me do more changes (next week).

### comment:127 Changed 5 months ago by git

• 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 git

• 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 git

• 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 slabbe

• 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 caruso

• Branch changed from u/slabbe/20343 to u/caruso/20343

### comment:132 Changed 5 months ago by slabbe

• Branch changed from u/caruso/20343 to u/slabbe/20343
• Commit changed from d4f4f426e4eb82df1ea2f96305ee14248b3d136b to 4e4b4a4a89c405e245b77bcb405b5ec9ab713627

New commits:

 ​9173d51 typos ​82ab15d 20343: adding experimental decorator to from_graph methods and removing the underscore prefix ​ddf2c39 20343: using check call to open the viewer ​fa55a01 20343: adding stackoverflow url ​4e4b4a4 20343: fixing a warning during docbuild

Needs review!

### comment:134 Changed 5 months ago by mkoeppe

+Loading few latex packages::


-> "a" few

### comment:135 Changed 5 months ago by mkoeppe

+#*****************************************************************************
+#       Copyright (C) 2015-2022 Sébastien Labbé <slabqc@gmail.com>
+#
+#
+#  The full text of the GPLv2 is available at:
+#


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 mkoeppe

+Adding a border in the options avoids croping the vertices of a graph::


-> cropping

### comment:137 follow-up: ↓ 144 Changed 5 months ago by mkoeppe

+        sage: t = TikzPicture(s, standalone_config=["border=4mm"], usepackage=['tkz-graph']) # 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 follow-up: ↓ 145 Changed 5 months ago by 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

### comment:139 follow-ups: ↓ 146 ↓ 148 Changed 5 months ago by mkoeppe

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 mkoeppe

Overall looking great!

### comment:141 Changed 5 months ago by slabbe

Thanks for your review! I will improve the code accordingly.

### comment:142 Changed 5 months ago by git

• Commit changed from 4e4b4a4a89c405e245b77bcb405b5ec9ab713627 to 3ed5d51a47ae8c3dd3c8090f33e73422b63d393f

Branch pushed to git repo; I updated commit sha1. New commits:

 ​7cf9af5 20343: fixing licence and typos ​3ed5d51 20343: adding an example showing usetikzlibrary in module doc

### comment:143 Changed 5 months ago by slabbe

It remains to fix the doctests tags. I will do this later.

### comment:144 in reply to: ↑ 137 Changed 5 months ago by slabbe

+        sage: t = TikzPicture(s, standalone_config=["border=4mm"], usepackage=['tkz-graph']) # 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)

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{tkz-graph}' 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 slabbe

+            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 slabbe

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 git

• Commit changed from 3ed5d51a47ae8c3dd3c8090f33e73422b63d393f to 36c3ebb5c2fa064b8c0f7e262ba2c3c06e9a9216

Branch pushed to git repo; I updated commit sha1. New commits:

 ​5f39c06 20343: adding latex and latex_package_tkz_graph tags where necessary ​8d981b6 20343:adding missing graphviz optional tag ​36c3ebb 20343: adding tag sage.graphs, sage.combinat, graphviz at some places

### comment:148 in reply to: ↑ 139 Changed 5 months ago by slabbe

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 git

• 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

Needs review!

### comment:151 follow-up: ↓ 152 Changed 5 months ago by mkoeppe

• Reviewers changed from Jean-Philippe Labbé, Xavier Caruso, Vincent Delecroix to Jean-Philippe Labbé, Xavier Caruso, Vincent Delecroix, Matthias Koeppe

Looking great.

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.

### comment:152 in reply to: ↑ 151 Changed 5 months ago by slelievre

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 mkoeppe

No, they wouldn't show up well in the plots.

### comment:154 follow-up: ↓ 157 Changed 5 months ago by caruso

• 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 git

• 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 slabbe

• 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 slabbe

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 slabbe

• Description modified (diff)

### comment:159 Changed 5 months ago by slabbe

• Description modified (diff)

### comment:160 Changed 5 months ago by slabbe

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 slelievre

• 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 slelievre

There must be a way to use tox for that...

### comment:163 Changed 5 months ago by slabbe

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 texlive-latex-base  $ lualatex -v

La commande « lualatex » n'a pas été trouvée, mais peut être installée avec :

sudo apt install texlive-latex-base

$convert -v La commande « convert » n'a pas été trouvée, mais peut être installée avec : sudo apt install graphicsmagick-imagemagick-compat # version 1.4+really1.3.35-1, or sudo apt install imagemagick-6.q16 # version 8:6.9.10.23+dfsg-2.1ubuntu11.4 sudo apt install imagemagick-6.q16hdri # version 8:6.9.10.23+dfsg-2.1ubuntu11.4  on which I installed sage: $ ./sage -version
SageMath version 9.6.beta5, Release Date: 2022-03-12


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 2022-03-24-11-08-32-82df3068.
Git branch: 20343
Using --optional=debian,pip,sage,sage_spkg
Doctesting 1 file.
sage -t --random-seed=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 2022-03-24-11-08-54-06354475.
Git branch: 20343
Using --optional=debian,external,pip,sage,sage_spkg
Doctesting 1 file.
sage -t --long --random-seed=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 caruso

• Status changed from needs_review to positive_review

Nice! I give back a positive review to this ticket.

Super!

### comment:166 Changed 4 months ago by vbraun

• 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 chapoton

• 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:169 follow-up: ↓ 171 Changed 2 months ago by 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/sage-support/c/1u4m-Osz6jY/m/PAM-1C40AwAJ?

### comment:170 Changed 2 months ago by jhpalmieri

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 slabbe

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/sage-support/c/1u4m-Osz6jY/m/PAM-1C40AwAJ?
No. The problem reported on sage-support 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.