sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.38k stars 471 forks source link

Fix error in sage.misc.latex.view doctest #34594

Closed jhpalmieri closed 2 years ago

jhpalmieri commented 2 years ago

With 9.8.beta0, the command

sage -t --long --optional=sage,optional,external src/sage/misc/latex.py

gives error.

Excerpted from #33931.

Component: misc

Author: John Palmieri, Sébastien Labbé

Branch/Commit: 81ff989

Reviewer: Sébastien Labbé, John Palmieri

Issue created by migration from https://trac.sagemath.org/ticket/34594

jhpalmieri commented 2 years ago

Branch: u/jhpalmieri/latex-error

jhpalmieri commented 2 years ago

New commits:

4b1d7cdtrac 34594: a doctest in "view" in sage.misc.latex should call
jhpalmieri commented 2 years ago

Commit: 4b1d7cd

seblabbe commented 2 years ago
comment:3

It seems we need to give the filename, not the file itself.

The command

sage -t --long --optional=sage,optional,external src/sage/misc/latex.py

gives

Using --optional=debian,external,glucose,pip,sage,sage_spkg
Features to be detected: 4ti2,benzene,bliss,buckygen,conway_polynomials,cplex,csdp,database_cremona_ellcurve,database_cremona_mini_ellcurve,database_cubic_hecke,database_jones_numfield,database_knotinfo,dvipng,ffmpeg,gfan,graphviz,gurobi,imagemagick,internet,jupymake,kenzo,latex,latex_package_tkz_graph,latte_int,lrslib,lualatex,macaulay2,magma,maple,mathematica,matlab,mcqd,meataxe,msolve,nauty,octave,palp,pandoc,pdf2svg,pdflatex,pdftocairo,phitigra,plantri,polytopes_db,polytopes_db_4d,pynormaliz,python_igraph,rubiks,sage.combinat,sage.geometry.polyhedron,sage.graphs,sage.groups,sage.misc.cython,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 --random-seed=49715222464713506332792189876546973295 src/sage/misc/latex.py
**********************************************************************
File "src/sage/misc/latex.py", line 1859, in sage.misc.latex.?
Failed example:
    with NamedTemporaryFile(mode="w+t", suffix=".tex") as f:  # optional - latex latex_package_tkz_graph
        _ = f.write(_latex_file_(g))
        f.flush()
        _run_latex_(f, engine="pdflatex")
Exception raised:
    Traceback (most recent call last):
      File "/home/slabbe/GitBox/sage/src/sage/doctest/forker.py", line 695, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/slabbe/GitBox/sage/src/sage/doctest/forker.py", line 1093, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.misc.latex.?[4]>", line 4, in <module>
        _run_latex_(f, engine="pdflatex")
      File "/home/slabbe/GitBox/sage/src/sage/misc/latex.py", line 771, in _run_latex_
        base, filename = os.path.split(filename)
      File "/home/slabbe/GitBox/sage/local/var/lib/sage/venv-python3.10.5/lib/python3.10/posixpath.py", line 103, in split
        p = os.fspath(p)
    TypeError: expected str, bytes or os.PathLike object, not _TemporaryFileWrapper
**********************************************************************
1 item had failures:
   1 of  19 in sage.misc.latex.?
    [257 tests, 1 failure, 1.64 s]
----------------------------------------------------------------------
sage -t --long --random-seed=49715222464713506332792189876546973295 src/sage/misc/latex.py  # 1 doctest failed
----------------------------------------------------------------------
Total time for all tests: 1.8 seconds
    cpu time: 0.3 seconds
    cumulative wall time: 1.6 seconds
Features detected for doctesting: imagemagick,latex,latex_package_tkz_graph
pytest is not installed in the venv, skip checking tests that rely on it
seblabbe commented 2 years ago

Reviewer: Sébastien Labbé

seblabbe commented 2 years ago

Changed commit from 4b1d7cd to 81ff989

seblabbe commented 2 years ago

Changed branch from u/jhpalmieri/latex-error to u/slabbe/34594

seblabbe commented 2 years ago
comment:4

f.name does the job


New commits:

81ff98934594: f -> f.name
seblabbe commented 2 years ago

Description changed:

--- 
+++ 
@@ -1,2 +1,9 @@
+With 9.8.beta0, the command
+
+```
+sage -t --long --optional=sage,optional,external src/sage/misc/latex.py
+```
+gives error.
+
 Excerpted from #33931.
jhpalmieri commented 2 years ago
comment:6

Yes, that's the right fix. Sorry I got it wrong.

jhpalmieri commented 2 years ago

Changed author from John Palmieri to John Palmieri, Sébastien Labbé

jhpalmieri commented 2 years ago

Changed reviewer from Sébastien Labbé to Sébastien Labbé, John Palmieri

seblabbe commented 2 years ago
comment:7

One patchbot shows issues with doc. The other not. I think the docbuild issue is unrelated to the current ticket.

jhpalmieri commented 2 years ago
comment:8

I agree.

vbraun commented 2 years ago

Changed branch from u/slabbe/34594 to 81ff989