Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some optional/not tested tags #18637

Closed
jdemeyer opened this issue Jun 8, 2015 · 18 comments
Closed

Fix some optional/not tested tags #18637

jdemeyer opened this issue Jun 8, 2015 · 18 comments

Comments

@jdemeyer
Copy link

jdemeyer commented Jun 8, 2015

Fix all # optional tags without a package name (except for giac which is a different ticket) and replace some # not tested tags by # known bug.

Component: doctest coverage

Author: Jeroen Demeyer

Branch/Commit: 8036f11

Reviewer: John Palmieri

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

@jdemeyer jdemeyer added this to the sage-6.8 milestone Jun 8, 2015
@jdemeyer
Copy link
Author

jdemeyer commented Jun 8, 2015

Branch: u/jdemeyer/ticket/18637

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 8, 2015

Commit: 23736ea

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 8, 2015

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

23736eaFix some optional/not tested tags

@jhpalmieri
Copy link
Member

comment:4

A few tests marked ImageMagick fail. Should we fix them here or elsewhere? These changes work for me:

diff --git a/src/sage/misc/latex.py b/src/sage/misc/latex.py
index ef76d09..6b47e20 100644
--- a/src/sage/misc/latex.py
+++ b/src/sage/misc/latex.py
@@ -1084,7 +1084,7 @@ class Latex(LatexCall):
             An error
             ...
             No pages of output.
-            <BLANKLINE>
+            ...
         """
         MACROS = latex_extra_preamble()
 
diff --git a/src/sage/plot/animate.py b/src/sage/plot/animate.py
index 83cb499..005dcde 100644
--- a/src/sage/plot/animate.py
+++ b/src/sage/plot/animate.py
@@ -118,7 +118,7 @@ import struct
 import zlib
 
 from sage.structure.sage_object import SageObject
-from sage.misc.temporary_file import tmp_dir, tmp_filename, graphics_filename
+from sage.misc.temporary_file import tmp_dir, tmp_filename
 import plot
 import sage.misc.misc
 import sage.misc.viewer
@@ -601,7 +601,7 @@ www.ffmpeg.org, or use 'convert' to produce gifs instead."""
                 raise OSError(msg)
         else:
             if not savefile:
-                savefile = graphics_filename(ext='.gif')
+                savefile = tmp_filename(ext='.gif')
             if not savefile.endswith('.gif'):
                 savefile += '.gif'
             savefile = os.path.abspath(savefile)
@@ -812,7 +812,7 @@ please install it and try again."""
                 else:
                     if output_format[0] != '.':
                         output_format = '.'+output_format
-                savefile = graphics_filename(ext=output_format)
+                savefile = tmp_filename(ext=output_format)
             else:
                 if output_format is None:
                     suffix = os.path.splitext(savefile)[1]
@@ -917,7 +917,7 @@ please install it and try again."""
         """
         pngdir = self.png()
         if savefile is None:
-            savefile = graphics_filename('.png')
+            savefile = tmp_filename('.png')
         with open(savefile, "wb") as out:
             apng = APngAssembler(
                 out, len(self),

@jhpalmieri
Copy link
Member

comment:5

Sorry, I missed this change:

diff --git a/src/sage/plot/animate.py b/src/sage/plot/animate.py
index 83cb499..a6bf569 100644
--- a/src/sage/plot/animate.py
+++ b/src/sage/plot/animate.py
@@ -43,8 +43,6 @@ Animate using FFmpeg_ instead of ImageMagick::
 Animate as an APNG_::
 
     sage: a.apng()  # long time
-    doctest:...: DeprecationWarning: use tmp_filename instead
-    See http://trac.sagemath.org/17234 for details.
 
 An animated :class:`sage.plot.graphics.GraphicsArray` of rotating ellipses::
 

@jdemeyer
Copy link
Author

jdemeyer commented Jun 8, 2015

comment:6

I prefer to move the changes graphics_filename -> tmp_filename to a different ticket, since that's not just about doctests, but about code.

@jhpalmieri
Copy link
Member

comment:7

Here are some other tests which could be fixed (they include extra commas):

databases/oeis.py:1640:            sage: f = oeis(45) ; f                      # optional -- internet, webbrowser
databases/oeis.py:1643:            sage: f.browse()                            # optional -- internet, webbrowser
rings/number_field/number_field.py:4738:            sage: NumberField(x^3 + 2*x + 1, 'a').galois_group(algorithm='magma')   # optional - magma, , database_gap
rings/polynomial/polynomial_rational_flint.pyx:1553:            sage: f.galois_group(algorithm='magma')  # optional - magma, database_gap

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 8, 2015

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

d63b0c9Minor optional doctest fixes

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 8, 2015

Changed commit from 23736ea to d63b0c9

@jhpalmieri
Copy link
Member

comment:9

Replying to @jdemeyer:

I prefer to move the changes graphics_filename -> tmp_filename to a different ticket, since that's not just about doctests, but about code.

I agree, especially since on further investigation, my changes don't work. Doctests pass, but the commands no longer behave correctly in the notebook.

Can we make the change to latex.py here? That's a very minor doctest fix.

@jdemeyer
Copy link
Author

jdemeyer commented Jun 8, 2015

comment:10

Replying to @jhpalmieri:

Can we make the change to latex.py here? That's a very minor doctest fix.

I don't have imagemagick installed and I don't want to install it for just this ticket. What's your output of ./sage -t --optional=all src/sage/misc/latex.py with the current branch (but without your changes)?

@jhpalmieri
Copy link
Member

comment:11

Replying to @jdemeyer:

Replying to @jhpalmieri:

Can we make the change to latex.py here? That's a very minor doctest fix.

I don't have imagemagick installed and I don't want to install it for just this ticket. What's your output of ./sage -t --optional=all src/sage/misc/latex.py with the current branch (but without your changes)?

There one long failed doctest, which I will abbreviate:

File "src/sage/misc/latex.py", line 1083, in sage.misc.latex.Latex.?
Failed example:
    latex.eval("\ThisIsAnInvalidCommand", {}) # optional -- ImageMagick
Expected:
    An error
    ...
    No pages of output.
    <BLANKLINE>
Got:
    An error occurred.
    This is pdfTeX, Version 3.14159265-2.6-1.40.15 (TeX Live 2014) (preloaded format=pdflatex 2014.9.22)  8 JUN 2015 12:34
    entering extended mode
     restricted \write18 enabled.
     %&-line parsing enabled.

        [snip]

    <BLANKLINE>
    (./sage43.aux) ) 
    Here is how much of TeX's memory you used:
     2758 strings out of 493108
     34433 string characters out of 6134847
     90768 words of memory out of 5000000
     6200 multiletter control sequences out of 15000+600000
     4403 words of font info for 15 fonts, out of 8000000 for 9000
     1141 hyphenation exceptions out of 8191
     38i,1n,26p,260b,38s stack positions out of 5000i,500n,10000p,200000b,80000s
    <BLANKLINE>
    No pages of output.
    PDF statistics:
     0 PDF objects out of 1000 (max. 8388607)
     0 named destinations out of 1000 (max. 500000)
     1 words of extra memory for PDF output out of 10000 (max. 10000000)
    <BLANKLINE>
    <BLANKLINE>
**********************************************************************
1 item had failures:
   1 of   2 in sage.misc.latex.Latex.?
    [308 tests, 1 failure, 12.37 s]
----------------------------------------------------------------------
sage -t src/sage/misc/latex.py  # 1 doctest failed
----------------------------------------------------------------------

So the issue is the extra lines starting "PDF statistics", plus two <BLANKLINE>'s at the end instead of one.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 8, 2015

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

8036f11Fix doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 8, 2015

Changed commit from d63b0c9 to 8036f11

@jdemeyer
Copy link
Author

jdemeyer commented Jun 8, 2015

comment:13

Does this work?

@jhpalmieri
Copy link
Member

comment:14

Yes, thank you. Everything else looks good, and I've checked some of them by installing the appropriate optional packages. Most of the changes are obviously the right things to do, so if they break some optional doctests, the tests were broken before. I don't have magma, maple, or mathematica installed, among others, so I have not tested everything, and any resulting broken optional doctests should probably be dealt with elsewhere.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@vbraun
Copy link
Member

vbraun commented Jun 9, 2015

Changed branch from u/jdemeyer/ticket/18637 to 8036f11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants