Update matplotlib to 2.1.0

[dimpase]

It's time we update; Debian already uses matplotlib 2.0.0.

For matplotlib 2.1.0 we need to update setuptools_scm and two new dependencies backports.functools_lru_cache and subprocess32.

Tarballs:

• ​http://sagetrac.lipn.univ-paris13.fr/sage/matplotlib-2.1.0.tar.bz2
• ​https://pypi.python.org/packages/4e/91/0e93d9455254b7b630fb3ebe30cc57cab518660c5fad6a08aac7908a4431/backports.functools_lru_cache-1.4.tar.gz
• ​https://pypi.python.org/packages/03/6d/aafdd01edd227ee879b691455bf19895091872af7e48192bea1758c82032/setuptools_scm-1.15.6.tar.gz
• ​http://ftp.swin.edu.au/gentoo/distfiles/subprocess32-3.2.7.tar.gz

[fbissey]

I'll have to look at debian's patches sometimes tomorrow. I backed out of 2.0.2 in sage-on-gentoo a few weeks ago

sage -t --long /usr/lib64/python2.7/site-packages/sage/plot/plot.py
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sage/plot/plot.py", line 483, in sage.plot.plot
Failed example:
    plt.savefig(os.path.join(SAGE_TMP, 'foo.png'))
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 518, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 888, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.plot.plot[77]>", line 1, in <module>
        plt.savefig(os.path.join(SAGE_TMP, 'foo.png'))
      File "/usr/lib64/python2.7/site-packages/matplotlib/pyplot.py", line 697, in savefig
        res = fig.savefig(*args, **kwargs)
      File "/usr/lib64/python2.7/site-packages/matplotlib/figure.py", line 1573, in savefig
        self.canvas.print_figure(*args, **kwargs)
      File "/usr/lib64/python2.7/site-packages/matplotlib/backend_bases.py", line 2252, in print_figure
        **kwargs)
      File "/usr/lib64/python2.7/site-packages/matplotlib/backends/backend_agg.py", line 545, in print_png
        FigureCanvasAgg.draw(self)
      File "/usr/lib64/python2.7/site-packages/matplotlib/backends/backend_agg.py", line 464, in draw
        self.figure.draw(self.renderer)
      File "/usr/lib64/python2.7/site-packages/matplotlib/artist.py", line 63, in draw_wrapper
        draw(artist, renderer, *args, **kwargs)
      File "/usr/lib64/python2.7/site-packages/matplotlib/figure.py", line 1144, in draw
        renderer, self, dsu, self.suppressComposite)
      File "/usr/lib64/python2.7/site-packages/matplotlib/image.py", line 139, in _draw_list_compositing_images
        a.draw(renderer)
      File "/usr/lib64/python2.7/site-packages/matplotlib/artist.py", line 63, in draw_wrapper
        draw(artist, renderer, *args, **kwargs)
      File "/usr/lib64/python2.7/site-packages/matplotlib/axes/_base.py", line 2426, in draw
        mimage._draw_list_compositing_images(renderer, self, dsu)
      File "/usr/lib64/python2.7/site-packages/matplotlib/image.py", line 139, in _draw_list_compositing_images
        a.draw(renderer)
      File "/usr/lib64/python2.7/site-packages/matplotlib/artist.py", line 63, in draw_wrapper
        draw(artist, renderer, *args, **kwargs)
      File "/usr/lib64/python2.7/site-packages/matplotlib/image.py", line 543, in draw
        renderer, renderer.get_image_magnification())
      File "/usr/lib64/python2.7/site-packages/matplotlib/image.py", line 770, in make_image
        unsampled=unsampled)
      File "/usr/lib64/python2.7/site-packages/matplotlib/image.py", line 426, in _make_image
        self.get_filternorm() or 0.0, self.get_filterrad() or 0.0)
    ValueError: 3-dimensional arrays must be of dtype unsigned byte, unsigned short, float32 or float64
**********************************************************************

sage -t --long /usr/lib64/python2.7/site-packages/sage/plot/arrow.py
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sage/plot/arrow.py", line 364, in sage.plot.arrow.Arrow._render_on_subplot
Failed example:
    two_stroke_re.search(contents) is None
Expected:
    True
Got:
    False
**********************************************************************

and

sage -t --long /usr/lib64/python2.7/site-packages/sage/probability/probability_distribution.pyx
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sage/probability/probability_distribution.pyx", line 120, in sage.probability.probability_distribution.ProbabilityDistribution.generate_histogram_data
Failed example:
    h, b = X.generate_histogram_data(bins = 10)
Expected nothing
Got:
    doctest:warning
      File "/usr/lib/python-exec/python2.7/sage-runtests", line 98, in <module>
        err = DC.run()
      File "/usr/lib64/python2.7/site-packages/sage/doctest/control.py", line 1099, in run
        self.run_doctests()
      File "/usr/lib64/python2.7/site-packages/sage/doctest/control.py", line 824, in run_doctests
        self.dispatcher.dispatch()
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 1777, in dispatch
        self.parallel_dispatch()
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 1667, in parallel_dispatch
        w.start()  # This might take some time
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 1943, in start
        super(DocTestWorker, self).start()
      File "/usr/lib64/python2.7/multiprocessing/process.py", line 130, in start
        self._popen = Popen(self)
      File "/usr/lib64/python2.7/multiprocessing/forking.py", line 126, in __init__
        code = process_obj._bootstrap()
      File "/usr/lib64/python2.7/multiprocessing/process.py", line 258, in _bootstrap
        self.run()
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 1916, in run
        task(self.options, self.outtmpfile, msgpipe, self.result_queue)
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 2218, in __call__
        result = runner.run(test)
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 663, in run
        return self._run(test, compileflags, out)
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 518, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 888, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.probability.probability_distribution.ProbabilityDistribution.generate_histogram_data[3]>", line 1, in <module>
        h, b = X.generate_histogram_data(bins = Integer(10))
      File "/usr/lib64/python2.7/site-packages/matplotlib/pyplot.py", line 3075, in hist
        mplDeprecation)
    :
    MatplotlibDeprecationWarning: The 'hold' keyword argument is deprecated since 2.0.
**********************************************************************

which is solved by #23696.

[dimpase]

Surely you meant #23689 (added to deps now).

[fbissey]

I was nicely self referential :) No patch for these in debian, they must live with it. I thought I had more stuff like the first failure, something silly like replacing [(0,0,0)] by [(0.0,0.0,0.0)] or some such fixes it if I remember correctly.

[jdemeyer]

I'm adding #23711 as dependency because it might be relevant to this ticket.

[fbissey]

Wondering about the test about arrow. From the file

        Dashed arrows should have solid arrowheads,
        :trac:`12852`. This test saves the plot of a dashed arrow to
        an EPS file. Within the EPS file, ``stroke`` will be called
        twice: once to draw the line, and again to draw the
        arrowhead. We check that both calls do not occur while the
        dashed line style is enabled::
            sage: a = arrow((0,0), (1,1), linestyle='dashed')
            sage: filename = tmp_filename(ext='.eps')
            sage: a.save(filename=filename)
            sage: with open(filename, 'r') as f:
            ....:     contents = f.read().replace('\n', ' ')
            sage: two_stroke_pattern = r'setdash.*stroke.*stroke.*setdash'
            sage: import re
            sage: two_stroke_re = re.compile(two_stroke_pattern)
            sage: two_stroke_re.search(contents) is None
            True

When I look at the result of the last one I get

sage: two_stroke_re.search(contents)
<_sre.SRE_Match object at 0x7f0420011bf8>

Obviously not none but we don't know from that if what is being tested failed or not. For the other doctest, it is a case of input being integer when the interface request a "real" type. Replacing a (0,0,0) by (0.0,0.0,0.0) does indeed fix the test.

[strogdon]

Replying to fbissey:

Wondering about the test about arrow. From the file

        Dashed arrows should have solid arrowheads,
        :trac:`12852`. This test saves the plot of a dashed arrow to
        an EPS file. Within the EPS file, ``stroke`` will be called
        twice: once to draw the line, and again to draw the
        arrowhead. We check that both calls do not occur while the
        dashed line style is enabled::
            sage: a = arrow((0,0), (1,1), linestyle='dashed')
            sage: filename = tmp_filename(ext='.eps')
            sage: a.save(filename=filename)
            sage: with open(filename, 'r') as f:
            ....:     contents = f.read().replace('\n', ' ')
            sage: two_stroke_pattern = r'setdash.*stroke.*stroke.*setdash'
            sage: import re
            sage: two_stroke_re = re.compile(two_stroke_pattern)
            sage: two_stroke_re.search(contents) is None
            True

When I look at the result of the last one I get

sage: two_stroke_re.search(contents)
<_sre.SRE_Match object at 0x7f0420011bf8>

Obviously not none but we don't know from that if what is being tested failed or not.

Not sure about what two_stroke_re.search(contents) returns, but I think the doctest actually failed. If I replace two_stroke_re.search(contents) is None with two_stroke_re.search(contents) is not None I get True. If you look at the generated .eps file the arrow is drawn with dashed lines. So I think the Sage patch from ticket #12852 is not working. I don't have matplotlib-2.1.0, 2.0.2 was used.

[fbissey]

Thanks Steve, I was working remotely so I didn't think of opening the generated .eps. I also use 2.0.2 right now but I am expecting 2.1.0 to be the same.

[strogdon]

It seems that line 440 in src/sage/plot/arrow.py

pe2 = ConditionalStroke(CheckNthSubPath(p, 1), [pe.Stroke(linestyle="solid")])

is doing nothing. In particular pe.Stroke() is ignoring linestyle="solid". This is the piece that insures the head of the arrow is filled/solid. With matplotlib-1.5.3 if I replace linestyle="solid" with linestyle="junk" I get ValueError: Unrecognized linestyle: junk when saving the .eps

            sage: a = arrow((0,0), (1,1), linestyle='dashed')
            sage: filename = tmp_filename(ext='.eps')
            sage: a.save(filename=filename)

I get no error when doing the same with matplotlib-2.0.2 and the head of the arrow is drawn with linestyle="dashed".

[strogdon]

The arrow issue has been reported upstream. See ​https://github.com/matplotlib/matplotlib/issues/9579

[strogdon]

Apparently a not so obvious API change. The following should work for >= 1.5.3. The .eps looks (needs to be confirmed!) OK.

--- a/src/sage/plot/arrow.py
+++ b/src/sage/plot/arrow.py
@@ -437,7 +437,7 @@ class Arrow(GraphicPrimitive):
                             pe1.draw_path(renderer, gc, tpath, affine, rgbFace)
 
             pe1 = ConditionalStroke(CheckNthSubPath(p, 0), [pe.Stroke()])
-            pe2 = ConditionalStroke(CheckNthSubPath(p, 1), [pe.Stroke(linestyle="solid")])
+            pe2 = ConditionalStroke(CheckNthSubPath(p, 1), [pe.Stroke(dashes={'dash_offset': 0, 'dash_list': None})])
             p.set_path_effects([pe1, pe2])
 
         subplot.add_patch(p)

The doctest ./sage -t --long src/sage/plot/arrow.py does pass on vanilla Sage with the above changes with matplotlib-1.5.3. However on sage-on-gentoo with matplotlib-2.0.2 the generated .eps looks OK but the test still fails?

sage -t --long --warn-long 119.2 /usr/lib/python2.7/site-packages/sage/plot/arrow.py
**********************************************************************
File "/usr/lib/python2.7/site-packages/sage/plot/arrow.py", line 364, in sage.plot.arrow.Arrow._render_on_subplot
Failed example:
    two_stroke_re.search(contents) is None
Expected:
    True
Got:
    False

[fbissey]

What does two_stroke_re.search(contents) says (similar to ​https://trac.sagemath.org/ticket/23696#comment:12)?

[strogdon]

Replying to fbissey:

What does two_stroke_re.search(contents) says (similar to ​https://trac.sagemath.org/ticket/23696#comment:12)?

Basically the same

sage: two_stroke_re.search(contents)
<_sre.SRE_Match object at 0x7fabe8b8e4a8>

[fbissey]

Time to inspect the .eps file and figure out what we are looking for. But first, lunch :)

[strogdon]

From what I have this setdash.*stroke.*stroke.*setdash is not matched in the .eps generated with version 1.5.3 but it is matched in the .eps generated with version 2.0.2. Nevertheless the .eps appears correct with version 2.0.2.

[strogdon]

In the .eps it appears that setdash is used with multiple stroke calls to draw the axis tics. So I think this is what is being found when setdash.*stroke.*stroke.*setdash is matched using version 2.0.2. I don't think the given matching mechanism is implementation proof.

[fbissey]

Yes, that's my conclusion too. So two choices:

1 Remove the test completely. That's rash, considering that enabled us to spot an interface change. 2 Work out something else to test. May not be possible.

[strogdon]

Try the following:

--- a/src/sage/plot/arrow.py
+++ b/src/sage/plot/arrow.py
@@ -354,14 +354,15 @@ class Arrow(GraphicPrimitive):
         dashed line style is enabled::
 
             sage: a = arrow((0,0), (1,1), linestyle='dashed')
+            sage: a.axes(False)
             sage: filename = tmp_filename(ext='.eps')
             sage: a.save(filename=filename)
             sage: with open(filename, 'r') as f:
             ....:     contents = f.read().replace('\n', ' ')
-            sage: two_stroke_pattern = r'setdash.*stroke.*stroke.*setdash'
+            sage: one_stroke_pattern = r'setdash.*stroke.*setdash.*stroke'
             sage: import re
-            sage: two_stroke_re = re.compile(two_stroke_pattern)
-            sage: two_stroke_re.search(contents) is None
+            sage: one_stroke_re = re.compile(one_stroke_pattern)
+            sage: one_stroke_re.search(contents) is not None
             True
         """
         from sage.plot.misc import get_matplotlib_linestyle
@@ -437,7 +438,7 @@ class Arrow(GraphicPrimitive):
                             pe1.draw_path(renderer, gc, tpath, affine, rgbFace)
 
             pe1 = ConditionalStroke(CheckNthSubPath(p, 0), [pe.Stroke()])
-            pe2 = ConditionalStroke(CheckNthSubPath(p, 1), [pe.Stroke(linestyle="solid")])
+            pe2 = ConditionalStroke(CheckNthSubPath(p, 1), [pe.Stroke(dashes={'dash_offset': 0, 'dash_list': None})])
             p.set_path_effects([pe1, pe2])
 
         subplot.add_patch(p)

So I've

1) removed the axes

2) make sure we match setdash.*stroke.*stroke.*setdash

We want to match the above since when successful the .eps looks something like

[7.4 3.2] 0 setdash
0.000 0.000 1.000 setrgbcolor
gsave
434.2 319 10.7 10.7 clipbox
19.049231 16.833846 m
158.203077 119.064615 296.756245 220.85409 434.708734 322.20227 c
stroke
grestore
[] 0 setdash
gsave
434.2 319 10.7 10.7 clipbox
423.689492 320.311157 m
434.708734 322.20227 l
429.610079 312.252209 l
423.689492 320.311157 l
cl
gsave
fill
grestore
stroke

The [] 0 setdash is the bit that draws the solid arrowhead. If that line is removed from the .eps file then the arrowhead is drawn with the dashed linetype. Of course, the description will have to be changed.

[fbissey]

Turning things on their head a little bit. Can we look for 0 setdash instead? Basically we draw a line with a 0 dash. So without changing the original eps, looking for something like

sage: zero_dashing_pattern = r'setdash.*stroke.*stroke.*0 setdash'
sage: zero_dashing_re = re.compile(zero_dashing_pattern)
sage: zero_dashing_re.search(contents) is not None

again it uses is not None which is not great.

[fbissey]

I have been trying a few things too. And I am getting frustrated because I fell like the doctest is now meaningless. It was introduced to check the behavior of matplotlib was conforming to some expectations, with an idea of before and after state. We have lost the "before" state which was in sage 5.10 (anyone wants to go back there?). In the meantime matplotlib behavior has changed and the output we are looking for doesn't make sense anymore.

I think we should just remove the doctest because we cannot make sense of what it should do anymore. It has become pointless and it is difficult to assert what we should test for now.

[strogdon]

It seems to me that the intent is to always draw arrows with solid arrowheads (perhaps for cosmetic reasons). This can be abandoned, I guess. However, if it is retained I believe this setdash.*setdash.*stroke.*stroke.*setdash matches when the arrowhead is drawn incorrectly but does not match when it is solid. This keeps the original .eps file.

[strogdon]

This would require that two lines be changed

--- a/src/sage/plot/arrow.py
+++ b/src/sage/plot/arrow.py
@@ -358,7 +358,7 @@ class Arrow(GraphicPrimitive):
             sage: a.save(filename=filename)
             sage: with open(filename, 'r') as f:
             ....:     contents = f.read().replace('\n', ' ')
-            sage: two_stroke_pattern = r'setdash.*stroke.*stroke.*setdash'
+            sage: two_stroke_pattern = r'setdash.*setdash.*stroke.*stroke.*setdash'
             sage: import re
             sage: two_stroke_re = re.compile(two_stroke_pattern)
             sage: two_stroke_re.search(contents) is None
@@ -437,7 +437,7 @@ class Arrow(GraphicPrimitive):
                             pe1.draw_path(renderer, gc, tpath, affine, rgbFace)
 
             pe1 = ConditionalStroke(CheckNthSubPath(p, 0), [pe.Stroke()])
-            pe2 = ConditionalStroke(CheckNthSubPath(p, 1), [pe.Stroke(linestyle="solid")])
+            pe2 = ConditionalStroke(CheckNthSubPath(p, 1), [pe.Stroke(dashes={'dash_offset': 0, 'dash_list': None})])
             p.set_path_effects([pe1, pe2])
 
         subplot.add_patch(p)

and perhaps some comments. This also works with version 1.5.3

[fbissey]

After a break I think you are right. One of the thing that bugs me is that the pattern searched for a bit too generic, it can catch a lot of things. The only thing you are sure of with the original pattern is that you look for at least two stroke (there could be even more) in between two setdash. I think we can try better by making sure we are in the right bounding box with

two_stroke_pattern = r'\[7\.4 3\.2\] 0 setdash.*stroke.*stroke.*setdash'

That ensure that we only look in that small box. And the result of the search should be negative. Of course I don't know if the values will stay the same across all systems or versions of matplotlib.

[fbissey]

Cross-posting, now I see you try to have two setdash first because there is a total of 3 (for now) so that get rid of the uncertainty about the box.

[fbissey]

Yes, with a few more comments I think it can go that way.

[fbissey]

All right, it is time to move ahead. Please review.

---

New commits:

​c35f93f	update matplotlib spkg. Use github in spkg-src instead of pypy (url more stable).
​d8630f8	matplotlib doesn't like integer in some context, and after 2.0, doesn't seem to cast them.
​ecf0cb7	fix arrow.py. 1) slight change of interface 2) in a doctest, improve the selection done by regular expression.
​6a0510f	Remove warning suppression needed with MPL-1.5/numpy-1.13

[dimpase]

it does seem to need libqhull, no?

[matplotlib-2.1.0]     building 'matplotlib._qhull' extension
[matplotlib-2.1.0]     gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -fPIC -DMPL_DEVNULL=/dev/null -DPY_ARRAY_UNIQUE_SYMBOL=MPL_matplotlib__qhull_ARRAY_API -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -D__STDC_FORMAT_MACROS=1 -I/home/dima/Sage/sage-dev/local/lib/python2.7/site-packages/numpy/core/include -I/home/dima/Sage/sage-dev/local/lib/python2.7/site-packages/numpy/core/include -I/home/dima/Sage/sage-dev/local/include -I. -I/home/dima/Sage/sage-dev/local/include/python2.7 -c src/qhull_wrap.c -o build/temp.linux-x86_64-2.7/src/qhull_wrap.o
[matplotlib-2.1.0]     gcc -pthread -shared -L/home/dima/Sage/sage-dev/local/lib -Wl,-rpath,/home/dima/Sage/sage-dev/local/lib -L/home/dima/Sage/sage-dev/local/lib -Wl,-rpath,/home/dima/Sage/sage-dev/local/lib build/temp.linux-x86_64-2.7/src/qhull_wrap.o -L/home/dima/Sage/sage-dev/local/lib -L/home/dima/Sage/sage-dev/local/lib64 -L/home/dima/Sage/sage-dev/local/lib -lqhull -lpython2.7 -o build/lib.linux-x86_64-2.7/matplotlib/_qhull.so
[matplotlib-2.1.0]     /usr/lib/gcc/x86_64-pc-linux-gnu/6.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: cannot find -lqhull
[matplotlib-2.1.0]     collect2: error: ld returned 1 exit status

[fbissey]

Ok, it should build its own included copy. More details please! At the beginning of the log there should be a configuration summary before it starts building proper, can you post that?

[fbissey]

That being said while it build successfully here I think some extra dependencies will need to be added.

[dimpase]

That's what I have on in Sage:

$ ls -l local/lib/*qhull*
-rw-r--r-- 1 dima dima 290024 Nov  7 23:15 local/lib/libqhullcpp.a
lrwxrwxrwx 1 dima dima     19 Nov  7 23:15 local/lib/libqhull_r.so -> libqhull_r.so.7.2.0
-rwxr-xr-x 1 dima dima 418128 Nov  7 23:15 local/lib/libqhull_r.so.7.2.0
-rw-r--r-- 1 dima dima 627426 Nov  7 23:15 local/lib/libqhullstatic.a
-rw-r--r-- 1 dima dima 618872 Nov  7 23:15 local/lib/libqhullstatic_r.a

Can one use that libqhull_r instead?

Note that qhull is an optional package. However, why does one need an included copy? Is it a latest trend, include qhull everywhere (IIRC, scipy also builds its own copy...)? :-)

here is the config summary:

    REQUIRED DEPENDENCIES AND EXTENSIONS
                     numpy: yes [version 1.13.3]
                       six: yes [using six version 1.11.0]
                  dateutil: yes [using dateutil version 2.5.3]
    backports.functools_lru_cache: yes [backports.functools_lru_cache was not
                            found. It is required forPython versions prior to
                            3.2]
              subprocess32: yes [subprocess32 was not found. It used  for Python
                            versions prior to 3.2 to improves functionality on
                            Linux and OSX]
                      pytz: yes [using pytz version 2016.4]
                    cycler: yes [using cycler version 0.10.0]
                   tornado: yes [using tornado version 4.3]
                 pyparsing: yes [using pyparsing version 2.1.5]
                    libagg: yes [pkg-config information for 'libagg' could not
                            be found. Using local copy.]
                  freetype: yes [version 2.6.3]
                       png: yes [version 1.6.29]
                     qhull: yes [Using system Qhull (version unknown, no pkg-
                            config info)]

by the way, I just installed a systemwide qhull (on gentoo), and the build works --- although that's not what we want here I suppose...

[fbissey]

That's not the latest trend. sage has not been in the habit of unbundling as much as possible unlike some distros. For scipy my experience in gentoo is worse. We tried unbundling some dependencies only to find they were not "pure", i.e. scipy had introduced stuff in the code to help hook it up.

In your case it looks like the sage optional qhull package may need updating or upgrading. I'll look into that latter. I suggest we make a separate ticket for it and depend on it here.

[dimpase]

to complete building Sage I also needed to do

./sage --pip install backports.functools_lru_cache

[fbissey]

Yes, as I was mentioning there will need dependencies added.

To go back to qhull, the fact that it works once you install it system wide probably point to a bug in matplotlib's build system. Definitely more stuff to look at.

[fbissey]

Nope, I must be wrong on qhull. Why does the spkg doesn't install libqhull.so? Because we use a basic (patched) makefile instead of using cmake to configure, build and install.

Minimum fix producing libqhull.so or write a .pc file so libqhull_r.so is used. We could do a link or write a file from spkg-install. What is your preferred solution?

[git]

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

​496ded5

add backports_functools_lru_cache a new dependency of MPL. Fix MPL 2.1.0 dependencies.

[fbissey]

Apart from the issues of qhull this should be all for the dependencies.

[strogdon]

Any obvious reason why this should be failing

Download error on https://pypi.python.org/simple/setuptools_scm/: [Errno 113] No route to host -- Some packages may not be found!
Download error on https://pypi.python.org/simple/setuptools-scm/: [Errno 113] No route to host -- Some packages may not be found!
Download error on https://pypi.python.org/simple/: [Errno 110] Connection timed out -- Some packages may not be found!
No local packages or working download links found for setuptools_scm>=1.15.0

The route looks good to me.

[strogdon]

To get this to work here I had to modify the existing setuptools_scm package, download setuptools_scm-1.15.6.tar.gz and install it manually.

[fbissey]

I think sage runs pip in a way it cannot download stuff. That's probably an unhelpful message. How did I get the stuff to install properly when all these problems exist.

TODO: update setuptools_scm in this ticket.

[fbissey]

Let's add subprocess32 I thought it was only optional from the message but it is probably better to have it.

[git]

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

​f6a9e53

update setuptools_scm for matplotlib-2.1.0

[git]

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

​717c3da

Add subprocess32 as a dependency of MPL-2.1.0

[fbissey]

There! Apart from the qhull issue that we should tackle separately, this is ready I believe.

[strogdon]

In doctesting I see two of

UserWarning: Attempted to set non-positive ylimits for log-scale axis; invalid limits will be ignored.

in src/sage/plot/plot.py which I haven't investigated. There is also numerous

UserWarning: The following kwargs were not used by contour: 'label'

where it appears that extra arguments in Sage calling functions (src/sage/plot/graphics.py has such a function) are incompatible with allowed extra arguments is matplotlib functions. This incompatibility has probably been around for some time but is now exposed by the addition of

        if kwargs:
            s = ", ".join(map(repr, kwargs))
            warnings.warn('The following kwargs were not used by contour: ' +
                          s)

in matplotlib/contour.py that prints the warning. If I remove the extra arguments for the Sage function calls, I don't get the warnings. This should all probably be revised/updated in some ticket.

[fbissey]

Ok, all our previous debugging was done on MPL-2.0.2 so it is fair that new things show up in 2.1.0. I should have done a doctest run.

[strogdon]

And there is ImportError: cannot import name delaunay in for example src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/introduction.rst. Delaunay may be gone. I remember something about delaunay deprecation issues in the past.

[fbissey]

Oh yes dear delaunay that I tried to replace only to find nothing else was like it. Was MPL-1.3 or 1.4? Well have to see if there is something it can be replaced with.

[fbissey]

OK in sage/plot.py` we have instances of semilog (one) and log-log (one) graph where a log axis start at 0. This is in listplot function where, if not provided, the index starts at 0 - a problem on a log scale. I will re-work those examples to have indices starting from 1.

In sage/plot/graphics.py one warning

MatplotlibDeprecationWarning: The set_linestyle function was deprecated in version 2.1.

Will need to dig. Also in sage/graphs/generic_graph.py, sage/manifolds/differentiable/vectorfield.py, sage/manifolds/differentiable/tangent_vector.py, sage/combinat/posets/posets.py, sage/plot/arrow.py,

will be fun to fix

grep -rI set_linestyle src/sage/*
src/sage/plot/ellipse.py:        p.set_linestyle(get_matplotlib_linestyle(options['linestyle'],return_type='long'))
src/sage/plot/arrow.py:        p.set_linestyle(get_matplotlib_linestyle(options['linestyle'], return_type='long'))
src/sage/plot/arrow.py:        p.set_linestyle(get_matplotlib_linestyle(options['linestyle'], return_type='long'))
src/sage/plot/circle.py:        p.set_linestyle(get_matplotlib_linestyle(options['linestyle'],return_type='long'))
src/sage/plot/arc.py:        p.set_linestyle(get_matplotlib_linestyle(options['linestyle'],
src/sage/plot/bezier_path.py:        bpatch.set_linestyle(get_matplotlib_linestyle(options['linestyle'], return_type='long'))
src/sage/plot/line.py:            p.set_linestyle(get_matplotlib_linestyle(options['linestyle'],

In sage/plot/plot3d/list_plot3d.py we have delaunay failures (13)

        from matplotlib import tri, delaunay
    ImportError: cannot import name delaunay

also in sage/stats/distributions/discrete_gaussian_lattice.py, en/thematic_tutorials/_sources/explicit_methods_in_number_theory/introduction.rst

In sage/plot/arrow.py strange deprecation warning to investigate

    MatplotlibDeprecationWarning: Adding an axes using the same arguments as a previous axes currently reuses the earlier instance.  In a future version, a new instance will always be created and returned.  Meanwhile, this warning can be suppressed, and the future behavior ensured, by passing a unique label to each axes instance.

In sage/plot/arc.py

sage -t --long --warn-long 89.4 /usr/lib64/python2.7/site-packages/sage/plot/arc.py
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sage/plot/arc.py", line 299, in sage.plot.arc.Arc.bezier_path
Failed example:
    b[0]
Expected:
    Bezier path from (1.618..., 0.5877...) to (-0.5176..., 0.9659...)
Got:
    Bezier path from (0.0, -1.0) to (0.0, -1.0)
**********************************************************************

In sage/probability/probability_distribution.pyx

File "/usr/lib64/python2.7/site-packages/sage/probability/probability_distribution.pyx", line 121, in sage.probability.probability_distribution.ProbabilityDistribution.generate_histogram_data
Failed example:
    h
Expected:
    [1.6299999999999999,
     0.0,
     0.0,
     0.0,
     0.0,
     1.9049999999999985,
     0.0,
     0.0,
     0.0,
     1.4650000000000003]
Got:
    [1.6300000000000003,
     0.0,
     0.0,
     0.0,
     0.0,
     1.9049999999999976,
     0.0,
     0.0,
     0.0,
     1.4650000000000012]
**********************************************************************

The last two are curious. I am noting them but deem them suspicious until further notice.

[strogdon]

This set_linestyle business is I believe what manifested itself with the arrow.py failure with version 2.0.2 and now throws a MatplotlibDeprecationWarning in other locations.

[fbissey]

Fun bit. The stuff plot.py with log plots is self inflicted. The example do it on purpose but MPL didn't display a user warning before.

[fbissey]

Replying to strogdon:

This set_linestyle business is I believe what manifested itself with the arrow.py failure with version 2.0.2 and now throws a MatplotlibDeprecationWarning in other locations.

Any idea has to what to use instead of set_linestyle? Or just skip it?

[git]

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

​2f4fed4	Merge branch 'develop' into matplotlib-2.1.0
​3508ea3	Add new user warnings in the expected output of a couple of tests

[fbissey]

Looks like you have to use a linestyle option in the object definition rather than setting it with a method. It will take a bit of time to sort all of them but I think I have the idea.

---

New commits:

​2f4fed4	Merge branch 'develop' into matplotlib-2.1.0
​3508ea3	Add new user warnings in the expected output of a couple of tests

[strogdon]

Replying to fbissey:

In sage/plot/graphics.py one warning

MatplotlibDeprecationWarning: The set_linestyle function was deprecated in version 2.1.

I don't get this in graphics.py. The only warning here is

sage -t --long src/sage/plot/graphics.py
**********************************************************************
File "src/sage/plot/graphics.py", line 1713, in sage.plot.graphics.Graphics.show
Failed example:
    p.show(gridlines=[[1,0],[-1,0,1]])
Expected nothing
Got:
    doctest:warning
...
    UserWarning: The following kwargs were not used by contour: 'label'

[fbissey]

I need to do a new fresh build.

[strogdon]

sage-on-gentoo with matplotlib-2.1.0 does give the MatplotlibDeprecationWarning but not the UserWarning. So there are some odd differences.

[strogdon]

I see matplotlib is in CDEPEND, so maybe that's it.

[fbissey]

It looks like I may something to migrate delaunay to. It looks like the default triangulation is delaunay now.

[git]

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

​2902b62	WIP: replace deprecated set_linestyle
​e4d8039	better (correct) fix for userwarnings in plot.py

[fbissey]

We need a "specialist" that knows what bezier_path is supposed to do. The change in behavior in arc.py is huge. We have gone from bezier_path drawing arcs to drawing full ellipses.

[git]

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

​7c028ad

easy fix

[fbissey]

OK the UserWarning: The following kwargs were not used by contour: 'label' can be easily fixed in and of itself, the problem is that it is looks like the tip of the iceberg of some stuff that receives little testing. inside contour_plot.py.

Some stuff in there about labels is rotting.

---

New commits:

​7c028ad

easy fix

[git]

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

​5d27a24

contour and contourf from matplotlib do not have a "label" keyword argument. The whole handling of labelling and legends may need a spruce up.

[strogdon]

Replying to git:

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

​5d27a24

contour and contourf from matplotlib do not have a "label" keyword argument. The whole handling of labelling and legends may need a spruce up.

Yes, I believe this is what I did to get rid of the the label warnings. And yes something is really wrong inside contour_plot.py. There is stuff there not consistent with even matplotlib-1.5.1, which someone needs to fix.

I'm curious, even though I think the set_linestyle needs to be addressed, I've never gotten a warning about it's use, that is with vanilla Sage.

[fbissey]

There are certainly inconsistencies between vanilla and gentoo but I am not completely sure why yet.

I think I got the set_linestyle that triggered the warnings but there are other all over the place. And I am not sure if there is still a function for that particular object or if it isn't tested. I added a linestyle argument to particular test and it didn't result in a warning so some objects haven't deprecated it yet.

I am puzzled by the stuff about axes being re-used. Re-used from where? I guess even if it isn't re-used it is about it needing some label. I have a feeling that it is MPL's fault rather than sage's fault.

The next biggie is delaunay. I may well not have it under control until Monday (cross finger I can do something about it).

[strogdon]

I have taken a look at delaunay. It's going to take some doing. In particular I can find no natural neighbor (nn) or spline options in the present triangulation. Or anything that would replicate what's presently done. But it may be hidden there somewhere.

[fbissey]

For the record at this point in time with this branch

----------------------------------------------------------------------
sage -t --long --warn-long 73.7 src/sage/stats/distributions/discrete_gaussian_lattice.py  # 1 doctest failed
sage -t --long --warn-long 73.7 src/sage/plot/arrow.py  # 1 doctest failed
sage -t --long --warn-long 73.7 src/sage/plot/plot3d/list_plot3d.py  # 13 doctests failed
sage -t --long --warn-long 73.7 src/sage/plot/arc.py  # 1 doctest failed
sage -t --long --warn-long 73.7 src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/introduction.rst  # 1 doctest failed
----------------------------------------------------------------------

17 doctest failures. 15 are failure to import delaunay and the other two are unique.

[strogdon]

This removes the MatplotlibDeprecationWarning in the src/sage/plot/arrow.py doctest

diff --git a/src/sage/plot/arrow.py b/src/sage/plot/arrow.py
index 2b293f17ca..fbce029306 100644
--- a/src/sage/plot/arrow.py
+++ b/src/sage/plot/arrow.py
@@ -336,7 +336,7 @@ class Arrow(GraphicPrimitive):
         this into account. See :trac:`12836`::
 
             sage: fig = Graphics().matplotlib()
-            sage: sp = fig.add_subplot(1,1,1)
+            sage: sp = fig.add_subplot(1,1,1, label='axis1')
             sage: a = arrow((0,0), (1,1))
             sage: b = arrow((0,0), (1,1), width=20)
             sage: p1 = a[0]._render_on_subplot(sp)

[fbissey]

Humpf, use label here, label doesn't exist there, can't they make up their mind.

[git]

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

​87b33cf

supress last warning in arrow.py by adding a label in the right place

[strogdon]

Replying to fbissey:

Humpf, use label here, label doesn't exist there, can't they make up their mind.

This is all very contorted (my opinion). Let's hope that the fix doesn't just cover up the real issue. But from Sage

sage: fig = Graphics().matplotlib()
sage: sp = fig.add_subplot(1,1,1, label='axis1')
sage: sp
<matplotlib.axes._subplots.AxesSubplot object at 0x7f1648115110>

So it would seem that add_subplot may call add_axes which then may require the label?

[fbissey]

Sounds like it. From the message given by MPL, that's clearly what they want you to do.

[strogdon]

To deal with delaunay we could always taint MPL-2.1.0 (or have an optional package) with the delaunay stuff from MPL-2.0.2. I have tried this and it works. But yet another item to maintain.

[strogdon]

From the matplotlib commit

+Code Removal
+````````````
+
+matplotlib.delaunay
+-------------------
+Remove the delaunay triangulation code which is now handled by Qhull
+via ``matplotlib.tri``

So I wonder if this is really correct?

[fbissey]

That's why I said I may have a migration path. This was definitely not the case when they first deprecated delaunay and I fought to find a replacement. So I am hopeful, but I am not working on that until tomorrow.

[fbissey]

OK now I remember the problem is not the triangulation, it is the "natural neighbor" interpolation going away. Solutions to that:

• get delaunay back (-1)
• get someone else's code (0)
• dropping "nearest neighbor" (0)

The last one wasn't very liked in #17618 2-3 years ago when upgrading to MPL-1.4.x. There is a couple of implementations floating around for natural neighbors. The original natgrid one that apparently has licensing issues, and I have seen at least another one with google but it may need a bit of work to "steal".

[strogdon]

Perhaps scipy ​here if one can figure out how to use it.

[fbissey]

Replying to strogdon:

Perhaps scipy ​here if one can figure out how to use it.

It wasn't good enough in #17618, however there is one new option that may improve things. I will have to dig the old code of #17618 to do some experiments.

[fbissey]

Ha, forgotten the scipy stuff is "nearest neighbor", the stuff from delaunay is "natural neighbor" and they are definitely different things.

All that's available has already been tried during the upgrade to MPL 1.4. See ​https://git.sagemath.org/sage.git/commit/?id=b2c0045d3619b225008984e7631c3abcf4f382e8 for various way of coding things without delaunay. There is nothing available unless we adopt code from somewhere else or write our own :(

[strogdon]

The documentation even confuses the terms nearest neighbor and natural neighbor.

[fbissey]

Replying to strogdon:

The documentation even confuses the terms nearest neighbor and natural neighbor.

In scipy, matplotlib or sage? I thought I had done a good effort during the MPL 1.4.x upgrade in sage.

[dimpase]

Replying to fbissey:

Replying to strogdon:

The documentation even confuses the terms nearest neighbor and natural neighbor.

In scipy, matplotlib or sage? I thought I had done a good effort during the MPL 1.4.x upgrade in sage.

Is it this one ​https://en.wikipedia.org/wiki/Natural_neighbor_interpolation ?

Surely Sage has more than one implementation of Voronoi/Delaunay stuff... Could you explain what exactly the issue is here?

[fbissey]

If there is, I'd like to know. The only I know about is the one that was in matplotlib's delaunay. Now under the name voronoi we may find other stuff.

[dimpase]

There is VoronoiDiagram in Sage, is it enough?

[strogdon]

Replying to fbissey:

Replying to strogdon:

The documentation even confuses the terms nearest neighbor and natural neighbor.

In scipy, matplotlib or sage? I thought I had done a good effort during the MPL 1.4.x upgrade in sage.

Yes, not clear. I was referring to the Sage documentation. In list_plot3d.py there is

    OPTIONAL KEYWORDS:

    - ``interpolation_type`` - 'linear', 'nn' (natural neighbor), 'spline'

and

      the triangulation. These points, the natural neighbors, are the ones 
      the interpolation point would connect to if inserted into the 
      triangulation.

and lastly

      When v is a matrix the default is to use linear interpolation, when
      v is a list of points the default is nearest neighbor.

[fbissey]

@steve OK. I forgot one.

@dima That's probably a start but I doubt that's quite enough. We would need to build the interpolator based on it.

[fbissey]

I am making experiments to time and see what various options look like. I used the same example than in #17618 but it looks slightly different even at the linear level. I don't know if it is the example or stuff in MPL 2.1.0 yet. I am doing a vanilla 8.1.rc0 build to be sure to have a good reference of before and after.

The spline looks bad - far worse than in #17618 and that's not based on MPL but on a scipy interpolator. The main problem is in the corners. There is spurious stuff trying to go to infinity in the corners which means the region that should be interesting in the middle of the graph is squashed. As always for this kind of things, it may be useful for some graphs.

I am also trialling ​https://docs.scipy.org/doc/scipy/reference/generated/scipy.interpolate.CloughTocher2DInterpolator.html#scipy.interpolate.CloughTocher2DInterpolator. It is fast (slightly faster than linear) and looks quite good.

I'll post again when I can see what it looks like with an identical build with MPL 1.5.3.

[fbissey]

spline are looking equally bad before and after the upgrade of MPL (since it is scipy based no surprise). Interestingly #17618 doesn't show what the spline result looks like so it is probably not new.

The linear results are different between MPL 1.5.3 and 2.1.0. If anything 2.1.0 looks more refined.

[fbissey]

So the code is basically

from sage.stats.distributions.discrete_gaussian_lattice import DiscreteGaussianDistributionLatticeSampler
D = DiscreteGaussianDistributionLatticeSampler(identity_matrix(2), 3.0)
S = [D() for _ in range(2^12)]
l = [vector(v.list() + [S.count(v)]) for v in set(S)]
%timeit P = list_plot3d(l, interpolation_type='linear', texture="automatic", point_list=True)
list_plot3d(l, interpolation_type='spline', texture="automatic", point_list=True)

with various replacement for interpolation type.

Note the variation in z across all examples.

MPL-1.5.3, linear

The slowest run took 20.64 times longer than the fastest. This could mean that an intermediate result is being cached.
100 loops, best of 3: 4.91 ms per loop

MPL-1.5.3, natural neighbor (nn)

100 loops, best of 3: 5.8 ms per loop

MPL-2.1.0, linear

The slowest run took 22.77 times longer than the fastest. This could mean that an intermediate result is being cached.
100 loops, best of 3: 4.96 ms per loop

MPL-2.1.0, CloughTocher? (new)

100 loops, best of 3: 4.6 ms per loop

I'd say MPL-2.1.0 linear looks much better than 1.5.3 and could be the new default. It is definitely smoother. We could also replace "natural neighbor" with "CloughTocher?" in the selection. It is an interesting one. As the fastest in the lot and not depending on some caching (unlike linear) it is a viable default in my opinion.

[strogdon]

With this somewhat naive change to list_plot3d.py added to this branch

diff --git a/src/sage/plot/plot3d/list_plot3d.py b/src/sage/plot/plot3d/list_plot3d.py
index c9f22dd464..dfc9aa8a51 100644
--- a/src/sage/plot/plot3d/list_plot3d.py
+++ b/src/sage/plot/plot3d/list_plot3d.py
@@ -373,7 +373,7 @@ def list_plot3d_tuples(v, interpolation_type, texture, **kwds):
         sage: list_plot3d([(1, 2, 3), (0, 1, 3), (2, 1, 4), (1, 0, -2)], texture='yellow', num_points=50)
         Graphics3d Object
     """
-    from matplotlib import tri, delaunay
+    from matplotlib import tri
     import numpy
     import scipy
     from random import random
@@ -441,16 +441,13 @@ def list_plot3d_tuples(v, interpolation_type, texture, **kwds):
 
     if interpolation_type == 'nn'  or interpolation_type =='default':
 
-        T=delaunay.Triangulation(x,y)
-        f=T.nn_interpolator(z)
+        T=tri.Triangulation(x,y)
+        f=tri.CubicTriInterpolator(T, z)
         f.default_value=0.0
         j=numpy.complex(0,1)
-        vals=f[ymin:ymax:j*num_points,xmin:xmax:j*num_points]
         from .parametric_surface import ParametricSurface
         def g(x,y):
-            i=round( (x-xmin)/(xmax-xmin)*(num_points-1) )
-            j=round( (y-ymin)/(ymax-ymin)*(num_points-1) )
-            z=vals[int(j),int(i)]
+            z = f(x, y)
             return (x,y,z)
         G = ParametricSurface(g, (list(numpy.r_[xmin:xmax:num_points*j]), list(numpy.r_[ymin:ymax:num_points*j])), texture=texture, **kwds)
         G._set_extra_kwds(kwds)

I get for the failing doctest in src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/introduction.rst

    sage: v = [[len(factor(n*m)) for n in [1..15]] for m in [1..15]]
    sage: list_plot3d(v, interpolation_type='nn')

the following

So with interpolation_type='nn' cubics are being used. This is to be compared with the same test but using MPL-1.5.3 (natural neighbor (nn) used) However, this change exposes DeprecationWarnings.

[fbissey]

For the record, this is what I used on top of this branch to get my graphs

src/sage/plot/plot3d/list_plot3d.py

@@ -373 +373 @@
         sage: list_plot3d([(1, 2, 3), (0, 1, 3), (2, 1, 4), (1, 0, -2)], texture='yellow', num_points=50)
         Graphics3d Object
     """
-    from matplotlib import tri, delaunay
+    from matplotlib import tri
     import numpy
     import scipy
     from random import random
@@ -427 +427 @@
                                           #arbitrary choice - assuming more or less a nxn grid of points
                                           # x should have n^2 entries. We sample 4 times that many points.
 
-    if interpolation_type == 'linear':
+    points=[[x[i],y[i]] for i in range(len(x))]
+    j = numpy.complex(0, 1)
+    
+    if interpolation_type == 'linear' or interpolation_type =='default':
         T = tri.Triangulation(x, y)
         f = tri.LinearTriInterpolator(T, z)
         j = numpy.complex(0, 1)
@@ -439 +442 @@
         G._set_extra_kwds(kwds)
         return G
 
-    if interpolation_type == 'nn'  or interpolation_type =='default':
-
-        T=delaunay.Triangulation(x,y)
-        f=T.nn_interpolator(z)
-        f.default_value=0.0
-        j=numpy.complex(0,1)
-        vals=f[ymin:ymax:j*num_points,xmin:xmax:j*num_points]
+    if interpolation_type == 'clough':
+        f = interpolate.CloughTocher2DInterpolator(points,z)
         from .parametric_surface import ParametricSurface
-        def g(x,y):
-            i=round( (x-xmin)/(xmax-xmin)*(num_points-1) )
-            j=round( (y-ymin)/(ymax-ymin)*(num_points-1) )
-            z=vals[int(j),int(i)]
-            return (x,y,z)
+        def g(x, y):
+            z = f([x, y])
+            return (x, y, z)
         G = ParametricSurface(g, (list(numpy.r_[xmin:xmax:num_points*j]), list(numpy.r_[ymin:ymax:num_points*j])), texture=texture, **kwds)
         G._set_extra_kwds(kwds)
         return G

[fbissey]

I didn't select cubic this time around because it was 16 times slower than natural neighbor in #17618. Of course that could have changed in 2.1.0.

[strogdon]

OK, I hadn't noticed the interpolate. business. Yet another twist.

[strogdon]

I think the only difference is that interpolate.CloughTocher2DInterpolator is on a regular grid while tri.CubicTriInterpolator is a Clough-Tocher cubic interpolation on a triangular grid, but I could be wrong.

[fbissey]

CloughTocher2DInterpolator is on unstructured data, so it can be any shape. Interestingly the input can be a delaunay triangulation, but it doesn't have to be on a grid. While it is a cubic interpolation, it is more sophisticated than tri.CubicTriInterpolator, in particular the scheme for evaluating the gradient (scipy) or derivatives (MPL) is vastly different.

[fbissey]

@ steve I just tested your example with my branch. in that case your "nn" picture is quite close to the linear from MPL 2.1.0. And the cubic picture is very similar to what I get for "clough". But I imagine cubic is much slower. Here are my timings

sage: v = [[len(factor(n*m)) for n in [1..15]] for m in [1..15]]
sage: %timeit NT=list_plot3d(v, interpolation_type='linear')
The slowest run took 325.09 times longer than the fastest. This could mean that an intermediate result is being cached.
10000 loops, best of 3: 83.4 µs per loop
sage: %timeit NT=list_plot3d(v, interpolation_type='clough')
100 loops, best of 3: 3.73 ms per loop

In this particular case linear is blazingly fast so may be cubic has a reasonable runtime.

[strogdon]

@françois, back to this.

My tests here indicate that CloughTocher2DInterpolator with your changes is roughly 20 times faster that CubicTriInterpolator. There are some subtle differences in the pictures so they are not exactly the same. A disadvantage of using CubicTriInterpolator is the DeprecationWarning mentioned above:

sage: v = [[len(factor(n*m)) for n in [1..15]] for m in [1..15]]
sage: %timeit NT=list_plot3d(v, interpolation_type='nn')
/64bitdev/storage/sage-git_develop/sage/local/lib/python2.7/site-packages/matplotlib/tri/triinterpolate.py:1074: DeprecationWarning: Both axis > a.ndim and axis < -a.ndim - 1 are deprecated and will raise an AxisError in the future.
  col0 = _prod_vectorized(J, np.expand_dims(tri_dz[:, 0, :], axis=3))
/64bitdev/storage/sage-git_develop/sage/local/lib/python2.7/site-packages/matplotlib/tri/triinterpolate.py:1075: DeprecationWarning: Both axis > a.ndim and axis < -a.ndim - 1 are deprecated and will raise an AxisError in the future.
  col1 = _prod_vectorized(J1, np.expand_dims(tri_dz[:, 1, :], axis=3))
/64bitdev/storage/sage-git_develop/sage/local/lib/python2.7/site-packages/matplotlib/tri/triinterpolate.py:1076: DeprecationWarning: Both axis > a.ndim and axis < -a.ndim - 1 are deprecated and will raise an AxisError in the future.
  col2 = _prod_vectorized(J2, np.expand_dims(tri_dz[:, 2, :], axis=3))
1 loop, best of 3: 241 ms per loop

interpolation_type='nn' does use cubics. If I change axis=3 to axis=2 in the 3 offending lines in matplotlib/tri/triinterpolate.py then the warnings disappear. I think there may be a mistake in the matplotlib code. There is this ​matplotlib pull that references this ​commit. But I'm not sure the changes were complete enough.

[fbissey]

Exact match is not required. Subtle differences are OK, it is bound to happen considering the differences in methods. I definitely think CloughTocher2D is more sophisticated than plain cubic and cubic are usually way nicer than linear. The factor 20 in speed is quite nice.

I am not going to comment on the deprecation warning but if you think that warrant an issue in matplotlib you should go for it.

[git]

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

​d29091b

Merge branch 'develop' into matplotlib-2.1.0

[git]

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

​594df56

MPL's delaunay module has been removed so we switch to regular triangulation (which happen to Delaunay by default in MPL) and replace Delaunay's 'natural neighbors'

[fbissey]

OK. Let's try to move this. I switched to the CloughTocher2D interpolator from scipy and hopefully updated all the documentation appropriately.

[egourgoulhon]

Thanks for the update!

However, with the new version atop Sage 8.2.beta0, I've noticed that the figures illustrating the "2D Plotting" section of the reference manual are of bad quality. For instance compare the attached plot-11.png (generated with MPL-2.1.0) with that at ​http://doc.sagemath.org/html/en/reference/plotting/sage/plot/plot.html

[egourgoulhon]

Another issue is the display of LaTeX in axes labels or text graphic objects; consider for instance

sage: text(r"$\sin(x^2) + \int_0^\pi \sqrt{x}\mathrm{d} x$", (1,1), fontsize=20)

With MPL-2.1.0, the output is

which is ugly, compared to what we have in Sage 8.1:

Also the size of the picture is different.

[strogdon]

It appears matplotlib is not finding the Computer Modern fonts. Here this seems to work

from matplotlib import rc
rc('text', usetex='True')
text(r"$\sin(x^2) + \displaystyle\int_0^\pi \sqrt{x}\mathrm{d} x$", (1,1), fontsize=20)

Here I need the \displaystyle to get the correct integral sign. And apparently matplotlib has to be told to use the TeX fonts.

@egourgoulhon does this work on your end?

[strogdon]

Actually, I think this is the desired result

sage: import matplotlib
sage: matplotlib.rcParams['mathtext.fontset']='cm'
sage: text(r"$\sin(x^2) + \int_0^\pi \sqrt{x}\mathrm{d} x$", (1,1), fontsize=20)

I think the default mathtext font is dejavusans.

[strogdon]

Replying to egourgoulhon:

Thanks for the update!

However, with the new version atop Sage 8.2.beta0, I've noticed that the figures illustrating the "2D Plotting" section of the reference manual are of bad quality. For instance compare the attached plot-11.png (generated with MPL-2.1.0) with that at ​http://doc.sagemath.org/html/en/reference/plotting/sage/plot/plot.html

So when I do

sage: G = plot(cos(x), (x, -5, 5), thickness=5, color='green', title='A plot')
sage: P = polygon([[1,2], [5,6], [5,0]], color='red')
sage: Q = polygon([(-x,y) for x,y in P[0]], color='blue')
sage: H = G + P + Q
sage: H

I get a .png that has very good quality but the size of the file containing the .png is about 45% larger than plot-11.png. So there are differences somewhere? I wonder what is generating the .png file?

[strogdon]

@fbissey

François, this might be a solution to the font issue (I don't want to mess up your branch):

build/pkgs/matplotlib/make-setup-config.py

@@ -9 +9 @@
 config.add_section('directories')
 config.set('directories', 'basedirlist', os.environ['SAGE_LOCAL'])
 
+config.add_section('rc_options')
+config.set('rc_options', 'mathtext.fontsel', 'cm')
 
 
 #####################################################################

[egourgoulhon]

Replying to strogdon:

It appears matplotlib is not finding the Computer Modern fonts. Here this seems to work

from matplotlib import rc
rc('text', usetex='True')
text(r"$\sin(x^2) + \displaystyle\int_0^\pi \sqrt{x}\mathrm{d} x$", (1,1), fontsize=20)

Here I need the \displaystyle to get the correct integral sign. And apparently matplotlib has to be told to use the TeX fonts.

@egourgoulhon does this work on your end?

No, I get the error message:

sage: from matplotlib import rc
sage: rc('text', usetex='True')
sage: text(r"$\sin(x^2) + \int_0^\pi \sqrt{x}\mathrm{d} x$", (1,1), fontsize=20)
/home/eric/sage/8.2.beta0/local/lib/python2.7/site-packages/sage/repl/rich_output/display_manager.py:590: RichReprWarning: Exception in _rich_repr_ while displaying object: [Errno 2] No such file or directory: 'dvipng'
  RichReprWarning,
Graphics object consisting of 1 graphics primitive

I guess this requires the package dvipng to be installed on my system (it is not).

[egourgoulhon]

Replying to strogdon:

Actually, I think this is the desired result

sage: import matplotlib
sage: matplotlib.rcParams['mathtext.fontset']='cm'
sage: text(r"$\sin(x^2) + \int_0^\pi \sqrt{x}\mathrm{d} x$", (1,1), fontsize=20)

This one works for me (i.e. it does not require dvipng).

[egourgoulhon]

Replying to strogdon:

So when I do

sage: G = plot(cos(x), (x, -5, 5), thickness=5, color='green', title='A plot')
sage: P = polygon([[1,2], [5,6], [5,0]], color='red')
sage: Q = polygon([(-x,y) for x,y in P[0]], color='blue')
sage: H = G + P + Q
sage: H

I get a .png that has very good quality but the size of the file containing the .png is about 45% larger than plot-11.png.

Indeed, this generates a good quality figure, contrary to what happens for the same commands when building the doc with Sphinx.

Besides, the resolution of the png is 630x464, while for the same plot, it is 784x575 in Sage 8.1.

[egourgoulhon]

One more data point: after make doc-clean + make doc, the png of 3D plots in the section 3D Graphics / Plotting Functions of the reference manual are also of bad quality. Now, these figures are not produced by matplotlib but by JMol... Could this be due to one of the new packages installed as MPL-2.1.0 dependencies?

[fbissey]

Sorry for the silence - I am sick. But now I am not lethargic anymore. I think changing things in make-setup-config.py is the way to go. It may be possible to change the size of the png file from there if it is desirable. I am not sure what's happening with jmol the new packages are python only and jmol is java. What could be possible, but I don't know, is that jmol calls matplotlib to generate its png files. In that case changing matplotlib's default should have an impact.

[git]

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

​b81b49c

Add Steve's suggestion to set the default fonts in matplotlib

[fbissey]

ok can you check again now?

[egourgoulhon]

Replying to fbissey:

ok can you check again now?

Thanks for the change, but it does not work: after rebuilding everything from scratch, the default font is still dejavusans.

By the way, I confirm that the low quality of all figures in the reference manual is due to this ticket and not to any other change introduced in Sage 8.2.beta0 (I mean, it is fine with 8.2.beta0 alone).

PS: I wish you a prompt recovery!

[egourgoulhon]

Replying to egourgoulhon:

By the way, I confirm that the low quality of all figures in the reference manual is due to this ticket and not to any other change introduced in Sage 8.2.beta0 (I mean, it is fine with 8.2.beta0 alone).

Maybe the code for sphinx_plot in src/doc/common/conf.py has to be updated. At present, it is

plot_pre_code = """
def sphinx_plot(plot):
    import matplotlib.image as mpimg
    from sage.misc.temporary_file import tmp_filename
    import matplotlib.pyplot as plt
    if os.environ.get('SAGE_SKIP_PLOT_DIRECTIVE', 'no') != 'yes':
        fn = tmp_filename(ext=".png")
        plot.plot().save(fn)
        img = mpimg.imread(fn)
        plt.imshow(img)
        plt.margins(0)
        plt.axis("off")
        plt.tight_layout(pad=0)

from sage.all_cmdline import *
"""

[fbissey]

Replying to egourgoulhon:

Replying to egourgoulhon:

By the way, I confirm that the low quality of all figures in the reference manual is due to this ticket and not to any other change introduced in Sage 8.2.beta0 (I mean, it is fine with 8.2.beta0 alone).

Maybe the code for sphinx_plot in src/doc/common/conf.py has to be updated. At present, it is

plot_pre_code = """
def sphinx_plot(plot):
    import matplotlib.image as mpimg
    from sage.misc.temporary_file import tmp_filename
    import matplotlib.pyplot as plt
    if os.environ.get('SAGE_SKIP_PLOT_DIRECTIVE', 'no') != 'yes':
        fn = tmp_filename(ext=".png")
        plot.plot().save(fn)
        img = mpimg.imread(fn)
        plt.imshow(img)
        plt.margins(0)
        plt.axis("off")
        plt.tight_layout(pad=0)

from sage.all_cmdline import *
"""

All right, to what? This code is only when generating plots for the doc. I am currently working on sage exclusively remotely, so I have no idea what the documentation looks like at the end. That's why I really need input from people who can see the results.

[egourgoulhon]

Replying to fbissey:

All right, to what? This code is only when generating plots for the doc. I am currently working on sage exclusively remotely, so I have no idea what the documentation looks like at the end. That's why I really need input from people who can see the results.

The sphinx_plot code is clearly the culprit: if you run the following code in a Jupyter notebook (not in Sage console, because plt.show() does not work there):

import matplotlib.image as mpimg
import matplotlib.pyplot as plt
fn = tmp_filename(ext=".png")
g = plot(sin(x^2), (x, 0, 4))
g.plot().save(fn)
img = mpimg.imread(fn)
plt.imshow(img)
plt.margins(0)
plt.axis("off")
plt.tight_layout(pad=0)
plt.show()

you get a low quality figure, while if you do the same thing with Sage 8.1, you get a nice figure.

By the way, this explains why 3D plots are also affected, because plot.plot() in sphinx_plot code transforms them into a 2D figure, which is then processed by mpimg and plt.

[egourgoulhon]

Replying to egourgoulhon:

The sphinx_plot code is clearly the culprit

In sphinx_plot code, changing

        plt.imshow(img)

to

        plt.imshow(img, interpolation='spline16')

gives a much better result, still on small size as compare to the previous version though.

The doc from plt.imshow? says:

interpolation : string, optional, default: None
   Acceptable values are 'none', 'nearest', 'bilinear', 'bicubic',
   'spline16', 'spline36', 'hanning', 'hamming', 'hermite', 'kaiser',
   'quadric', 'catrom', 'gaussian', 'bessel', 'mitchell', 'sinc',
   'lanczos'

   If interpolation is None, default to rc image.interpolation. See
   also the filternorm and filterrad parameters. If interpolation is
   'none', then no interpolation is performed on the Agg, ps and pdf
   backends. Other backends will fall back to 'nearest'.

So maybe something around rc image.interpolation should be worked out.

[strogdon]

Replying to egourgoulhon:

Replying to fbissey:

ok can you check again now?

Thanks for the change, but it does not work: after rebuilding everything from scratch, the default font is still dejavusans.

There is a typo in what I suggested. Not sure what I did, given the typo, to make the font selection work. The recent commit should change

config.set('rc_options', 'mathtext.fontsel', 'cm')

to

config.set('rc_options', 'mathtext.fontset', 'cm')

fontsel -> fontset

But even with the change, I'm unable to get matplotlib to recognize the font change. Something is wrong.

[strogdon]

Perhaps the font can't be changed unless changed in matplotlibrc before the install? This doesn't seem right. However, If I insert mathtext.fontset : cm in local/lib/python2.7/site-packages/matplotlib/mpl-data/matplotlibrc things seem to work.

[fbissey]

Thanks for all that info! I will dig into this slowly. I am still officially "unfit for work" (doctor's order) but a bit of activity at this stage should be alright.

[fbissey]

All right. make-setup-config.py was a nice idea, but it doesn't have an impact on the final mpl-data/matplotlibrc. So if we want to play with that because we think the default sucks, we'll have to tweak it at some stage. I'd like to do it post-install but is it hard to do it right for all python. It is better to patch the default one.

So the branch will be reset one commit behind and then I'll add new stuff.

I am starting to compare mpl-data/matplotlibrc for interesting differences. For example image.interpolation was bilinear in 1.5.3 and is nearest in 2.1.0. Something for you to check Eric.

[charpent]

I'm pretty interested by this one. I'm trying to implement the "exclusions" never implemented in parametric_plot3d, and I might be reinventing the wheel...

[fbissey]

Replying to charpent:

I'm pretty interested by this one. I'm trying to implement the "exclusions" never implemented in parametric_plot3d, and I might be reinventing the wheel...

Feel free to dig around and interface anything you find. However I can hear Jeroen complaining that it should have been done in a separate ticket on top of that one. But I hope you find what you want in this release. It looks like 2.1.x may be the latest branch to support python 2.7 by the way (unless I misunderstood and 2.2.0 will be it).

[fbissey]

My reading on the default matplotlibrc: transparency is off by default, not sure that's a big impact. Lines are thicker by default (by 0.5 points). Fonts are smaller by default (10 points instead of 12). Some more on fonts (- is 1.5.3, + is 2.1.0)

-#font.size           : 12.0
-#font.serif          : Bitstream Vera Serif, New Century Schoolbook, Century Schoolbook L, Utopia, ITC Bookman, Bookman, Nimbus Roman No9 L, Times New Roman, Times, Palatino, Charter, serif
-#font.sans-serif     : Bitstream Vera Sans, Lucida Grande, Verdana, Geneva, Lucid, Arial, Helvetica, Avant Garde, sans-serif
+#font.size           : 10.0
+#font.serif          : DejaVu Serif, Bitstream Vera Serif, New Century Schoolbook, Century Schoolbook L, Utopia, ITC Bookman, Bookman, Nimbus Roman No9 L, Times New Roman, Times, Palatino, Charter, serif
+#font.sans-serif     : DejaVu Sans, Bitstream Vera Sans, Lucida Grande, Verdana, Geneva, Lucid, Arial, Helvetica, Avant Garde, sans-serif
 #font.cursive        : Apple Chancery, Textile, Zapf Chancery, Sand, Script MT, Felipa, cursive
-#font.fantasy        : Comic Sans MS, Chicago, Charcoal, Impact, Western, Humor Sans, fantasy
-#font.monospace      : Bitstream Vera Sans Mono, Andale Mono, Nimbus Mono L, Courier New, Courier, Fixed, Terminal, monospace
+#font.fantasy        : Comic Sans MS, Chicago, Charcoal, Impact, Western, Humor Sans, xkcd, fantasy
+#font.monospace      : DejaVu Sans Mono, Bitstream Vera Sans Mono, Andale Mono, Nimbus Mono L, Courier New, Courier, Fixed, Terminal, monospace

Mainly DejaVu has displaced Bitstream Vera as the default. But more importantly in a later section

-#mathtext.fontset : cm # Should be 'cm' (Computer Modern), 'stix',
-                       # 'stixsans' or 'custom'
+#mathtext.fontset : dejavusans # Should be 'dejavusans' (default),
+                               # 'dejavuserif', 'cm' (Computer Modern), 'stix',
+                               # 'stixsans' or 'custom'

So upstream really thinks DejaVu is superior. We may be considered backward in our preferences soon. I am for bigger fonts myself as a short sighted person. I suspect the following is causing the figures to be smaller - If someone wants to check

 ### FIGURE
 # See http://matplotlib.org/api/figure_api.html#matplotlib.figure.Figure
-#figure.titlesize : medium     # size of the figure title
+#figure.titlesize : large      # size of the figure title (Figure.suptitle())
 #figure.titleweight : normal   # weight of the figure title
-#figure.figsize   : 8, 6    # figure size in inches
-#figure.dpi       : 80      # figure dots per inch
-#figure.facecolor : 0.75    # figure facecolor; 0.75 is scalar gray
+#figure.figsize   : 6.4, 4.8   # figure size in inches
+#figure.dpi       : 100      # figure dots per inch
+#figure.facecolor : white   # figure facecolor; 0.75 is scalar gray
 #figure.edgecolor : white   # figure edgecolor
 #figure.autolayout : False  # When True, automatically adjust subplot

notice that the figure is smaller (figsize) but the resolution is greater (dpi). And that's it. The rest is more fluffy compared to what we use normally.

[fbissey]

We can bluntly override the default or create a sage style-sheet ​https://matplotlib.org/users/customizing.html and drop it under mpl-data/stylelib/ (where upstream is already storing them). The alternative is under MPLCONFIGDIR and it is not palatable because it is an individual user location. We could even submit our style for inclusion upstream.

[fbissey]

There is a "classic" style, classic being MPL-1.5. I'll just push a little change and if I am not mistaken the doc will just look as before.

[git]

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

​29a35c2

use the classic MPL 1.5 style in sphinx_plot

[egourgoulhon]

Replying to fbissey:

So upstream really thinks DejaVu is superior. We may be considered backward in our preferences soon.

Sage being mathematics software, we should set cm as the default font, whatever upstream thinks. Mathematical formulas with dejavusans look really ugly and cm has the big advantage of making labels/legends of figures for a scientific paper/textbook coincide with those of the main text.

I am for bigger fonts myself as a short sighted person.

+1; so for this one, we should keep MPL-2.1.0 defaults.

[fbissey]

Replying to egourgoulhon:

Replying to fbissey:

So upstream really thinks DejaVu is superior. We may be considered backward in our preferences soon.

Sage being mathematics software, we should set cm as the default font, whatever upstream thinks. Mathematical formulas with dejavusans look really ugly and cm has the big advantage of making labels/legends of figures for a scientific paper/textbook coincide with those of the main text.

I am for bigger fonts myself as a short sighted person.

+1; so for this one, we should keep MPL-2.1.0 defaults.

Fonts are bigger in MPL 1.5.3! They are smaller in 2.1.0. But using "classic" should take care of that too.

[egourgoulhon]

Replying to fbissey:

Replying to egourgoulhon:

I am for bigger fonts myself as a short sighted person.

+1; so for this one, we should keep MPL-2.1.0 defaults.

Fonts are bigger in MPL 1.5.3! They are smaller in 2.1.0.

Sorry, I misunderstood. I agree with you: bigger fonts are better.

[charpent]

Replying to fbissey:

Replying to charpent:

I'm pretty interested by this one. I'm trying to implement the "exclusions" never implemented in parametric_plot3d, and I might be reinventing the wheel...

Feel free to dig around and interface anything you find. However I can hear Jeroen complaining that it should have been done in a separate ticket on top of that one. But I hope you find what you want in this release. It looks like 2.1.x may be the latest branch to support python 2.7 by the way (unless I misunderstood and 2.2.0 will be it).

Huh... Jeroen would be right ... if I intended to suggest anything exclusions-related to the present ticket, which is not the case. It is another problem, indeed...

My idea was as follows : I found that plot3d and friends leaved a "hole" in the plotted surface when a vertex of a rectangular facet had float("NaN") as z-coordinate. This can be pretty useful to represent exclusions (another way is to use implicit_plot3d, which is pretty costly in terms of computing time (proportional to resolution cubed, practically unusable....)). So I wanted to follow this to see if it entailed changes relevant to this.

Anyway, I now think that I'm running in a dead-end. My idea bumps on a pretty interesting list_plot3d snag... More on this in a future ticket (if I can exactly corner the problem...).

[egourgoulhon]

Replying to fbissey:

There is a "classic" style, classic being MPL-1.5. I'll just push a little change and if I am not mistaken the doc will just look as before.

Yes I confirm it does look the same. However, this solution (i.e. setting plt.style.use('classic') in the code of sphinx_plot) may not be a good one, because the figures in the doc may differ from what the user get by typing the same commands, which is not desirable I guess. For instance, the doc figures are different from those produced in the console at the moment, because of the font issue.

[strogdon]

Replying to egourgoulhon:

Replying to fbissey:

There is a "classic" style, classic being MPL-1.5. I'll just push a little change and if I am not mistaken the doc will just look as before.

Yes I confirm it does look the same. However, this solution (i.e. setting plt.style.use('classic') in the code of sphinx_plot) may not be a good one, because the figures in the doc may differ from what the user get by typing the same commands, which is not desirable I guess. For instance, the doc figures are different from those produced in the console at the moment, because of the font issue.

Ditto here. After a make doc-clean && make the figures in the docs (at least the ones I looked at) appear to be OK. But the documentation example to illustrate use of TeX fonts

sage: plot(exp(x), 0, 2, legend_label='$e^x$')

does not typeset the label with TeX fonts in the terminal. So the mathtext.fontset from build/pkgs/matplotlib/make-setup-config.py needs to be removed since it's not working there and the equivalent needs to be put somewhere. Where I don't know. And then let's hope that that doesn't mess with plt.style.use('classic').

[fbissey]

I removed mathtext.fontset from make-setup-config.py in the last commit. I didn't think it was useful. So you could make the change to see if it has an impact for you. If it does, I can include it properly.

Now, Eric's comment about reproducibility is quite serious and probably a lot of work. The easiest solution is of course to just copy the classic style as matplotlibrc in the source and may be make a copy of the original one as a MPL-2.1.0 style. sage-on-distro people will rise to linch, verbally - we are quite civilized really [yes I am part of that mob]. So the alternative is documentation, documentation and more documentation :( It has to show up in examples that people would follow with an explanation why. There is currently 668 PLOT:: directives in sage's source code who's having fun yet?

[strogdon]

Replying to fbissey:

I removed mathtext.fontset from make-setup-config.py in the last commit. I didn't think it was useful. So you could make the change to see if it has an impact for you. If it does, I can include it properly.

I hadn't merged things properly and it was still there, but now it's gone.

Now, Eric's comment about reproducibility is quite serious and probably a lot of work. The easiest solution is of course to just copy the classic style as matplotlibrc in the source and may be make a copy of the original one as a MPL-2.1.0 style. sage-on-distro people will rise to linch, verbally - we are quite civilized really [yes I am part of that mob]. So the alternative is documentation, documentation and more documentation :( It has to show up in examples that people would follow with an explanation why. There is currently 668 PLOT:: directives in sage's source code who's having fun yet?

I misunderstood. There are probably other items besides fonts that can show up if the classic style isn't available from the terminal. But here putting the mathtext.fontset declaration in matplotlibrc seems to resolve the font issue.

[fbissey]

That's one of things a pre-made style help with. Unless you really, really think upstream screwed up with their choice of math fonts (I cannot really defend them myself).

[dimpase]

Replying to egourgoulhon:

Replying to fbissey:

There is a "classic" style, classic being MPL-1.5. I'll just push a little change and if I am not mistaken the doc will just look as before.

Yes I confirm it does look the same. However, this solution (i.e. setting plt.style.use('classic') in the code of sphinx_plot) may not be a good one, because the figures in the doc may differ from what the user get by typing the same commands, which is not desirable I guess. For instance, the doc figures are different from those produced in the console at the moment, because of the font issue.

I think trying to make docs look exactly as online help is doomed to fail, in particular as online may mean

• terminal (with figures shown by a dedicated viewer),
• terminal - figures in the browser
• Jupyter
• sagenb
• ...

[egourgoulhon]

Replying to dimpase:

I think trying to make docs look exactly as online help is doomed to fail, in particular as online may mean

• terminal (with figures shown by a dedicated viewer),
• terminal - figures in the browser
• Jupyter
• sagenb
• ...

This may be true for 3D plots (which are not produced by matplotplib, by the way), but not for 2D plots generated by matplotlib: they all end up in a png file and all viewers will display this png figure in a unique way, whatever backend you are using (console, Jupyter, sagenb, CoCalc, SageMathCell, ...)

[strogdon]

I now have the following failures:

sage -t --long src/sage/plot/arc.py
sage -t --long src/sage/plot/plot3d/list_plot3d.py
sage -t --long src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/introduction.rst

sage -t --long src/sage/plot/arc.py
**********************************************************************
File "src/sage/plot/arc.py", line 299, in sage.plot.arc.Arc.bezier_path
Failed example:
    b[0]
Expected:
    Bezier path from (1.618..., 0.5877...) to (-0.5176..., 0.9659...)
Got:
    Bezier path from (0.0, -1.0) to (0.0, -1.0)
**********************************************************************
1 item had failures:
   1 of   7 in sage.plot.arc.Arc.bezier_path
    [47 tests, 1 failure, 6.75 s]
----------------------------------------------------------------------
sage -t --long src/sage/plot/arc.py  # 1 doctest failed
----------------------------------------------------------------------

sage -t --long src/sage/plot/plot3d/list_plot3d.py
**********************************************************************
File "src/sage/plot/plot3d/list_plot3d.py", line 95, in sage.plot.plot3d.list_plot3d.list_plot3d
Failed example:
    list_plot3d(m, texture='yellow', interpolation_type='nn', frame_aspect_ratio=[1, 1, 1/3])
Expected:
    Graphics3d Object
Got:
    <BLANKLINE>
**********************************************************************
File "src/sage/plot/plot3d/list_plot3d.py", line 102, in sage.plot.plot3d.list_plot3d.list_plot3d
Failed example:
    list_plot3d(m, texture='yellow', interpolation_type='nn', frame_aspect_ratio=[1, 1, 1/3], num_points=40)
Expected:
    Graphics3d Object
Got:
    <BLANKLINE>
**********************************************************************
File "src/sage/plot/plot3d/list_plot3d.py", line 143, in sage.plot.plot3d.list_plot3d.list_plot3d
Failed example:
    list_plot3d(l, interpolation_type='nn', texture='yellow', num_points=100)
Expected:
    Graphics3d Object
Got:
    <BLANKLINE>
**********************************************************************
1 item had failures:
   3 of  26 in sage.plot.plot3d.list_plot3d.list_plot3d
    [39 tests, 3 failures, 18.32 s]
----------------------------------------------------------------------
sage -t --long src/sage/plot/plot3d/list_plot3d.py  # 3 doctests failed
----------------------------------------------------------------------

sage -t --long src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/introduction.rst
**********************************************************************
File "src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/introduction.rst", line 77, in doc.en.thematic_tutorials.explicit_methods_in_number_theory.introduction
Failed example:
    list_plot3d(v, interpolation_type='nn')
Exception raised:
    Traceback (most recent call last):
      File "/64bitdev/storage/sage-git_develop/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 517, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/64bitdev/storage/sage-git_develop/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 920, in compile_and_execute
        exec(compiled, globs)
      File "<doctest doc.en.thematic_tutorials.explicit_methods_in_number_theory.introduction[3]>", line 1, in <module>
        list_plot3d(v, interpolation_type='nn')
      File "/64bitdev/storage/sage-git_develop/sage/local/lib/python2.7/site-packages/sage/plot/plot3d/list_plot3d.py", line 212, in list_plot3d
        return list_plot3d_array_of_arrays(v, interpolation_type, texture, **kwds)
      File "/64bitdev/storage/sage-git_develop/sage/local/lib/python2.7/site-packages/sage/plot/plot3d/list_plot3d.py", line 296, in list_plot3d_array_of_arrays
        G._set_extra_kwds(kwds)
    AttributeError: 'NoneType' object has no attribute '_set_extra_kwds'
**********************************************************************
1 item had failures:
   1 of   9 in doc.en.thematic_tutorials.explicit_methods_in_number_theory.introduction
    [6 tests, 1 failure, 2.22 s]
----------------------------------------------------------------------
sage -t --long src/doc/en/thematic_tutorials/explicit_methods_in_number_theory/introduction.rst  # 1 doctest failed
----------------------------------------------------------------------

Most are related to additional interpolation_type='nn'. And then there is the Bezier business.

[fbissey]

I had completely forgotten about the Bezier stuff in arc.py. I don't know what to do with that, the behavior is so completely different. The rest like like stuff I overlook.

[git]

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

​91749f5	Merge branch 'develop' into matplotlib-2.1.0
​8f4ca6b	convert all instances of interpolation='nn' to 'clough'

[fbissey]

OK should only be the Bezier stuff in arc.py. If someone has a clue has to what needs to be done there. The issue is that previously just arcs would be plotted while now we get whole ellipses. And this is why the numerical value are so different - the values are for coordinates from the start to the end of the arc. As you can see with MPL-2.1.0 the beginning is equal to the end.

So there is probably an interface to tweak, so we get only arcs again.

[strogdon]

This is the pull ​request that was merged which breaks the arc.py doctest.

[fbissey]

Lovely, if I read this correctly, they fixed a "wrong" behavior and sage may have been working around it - or possibly using it as a feature, that's not unprecedented - for some time. Also I notice 2.1.1 is released, may as well bump it.

[strogdon]

I think the problem may well be with bezier_path. This generates an arc

sage: a = arc((0,0),2,1,0,(pi/5,pi/2+pi/12))
sage: a

However this generates an ellipse

sage: b=a[0].bezier_path()
sage: b

and

sage: a[0]
Arc with center (0.0,0.0) radii (2.0,1.0) angle 0.0 inside the sector (0.628318530718,1.83259571459)

[fbissey]

Yup. I am still trying to work out how bezier_path is supposed to work. It is not exactly well explained, I had to track matplotlib classes that were not obvious. That's getting me back to high school and some stuff in first year. But why we do it, is a mystery so far. And there may be a matplotlib class that give you the same stuff, although there may be issue of it being a different class. Finally, I am unclear the stuff is ever used (even as it says it is needed for some other stuff).

[fbissey]

@ chapoton I am adding you to this ticket because you added bezier_path in plot/arc.py in #18939. Upgrading matplotlib to 2.1.0 makes the bezier_path whole ellipses rather that arcs. Steve Trogdon pointed out that ​a ​https://github.com/matplotlib/matplotlib/pull/8047 corrected the proportions in matplotlib and is the breaking commit. Could you advise on how to change your code?

[strogdon]

For discussion only. I'm sure all corner cases have not been addressed:

src/sage/plot/arc.py

@@ -301 +301 @@
         """
         from sage.plot.bezier_path import BezierPath
         from sage.plot.graphics import Graphics
+        from matplotlib.path import Path
+        import numpy as np
         ma = self._matplotlib_arc()
-        transform = ma.get_transform().get_matrix()
+        def theta_stretch(theta, scale):
+            theta = np.deg2rad(theta)
+            x = np.cos(theta)
+            y = np.sin(theta)
+            return np.rad2deg(np.arctan2(scale * y, x))
+        theta1 = theta_stretch(ma.theta1, ma.width / ma.height)
+        theta2 = theta_stretch(ma.theta2, ma.width / ma.height)
+
+        pa = ma
+        pa._path = Path.arc(theta1, theta2)
+        transform = pa.get_transform().get_matrix()
         cA, cC, cE = transform[0]
         cB, cD, cF = transform[1]
         points = []
-        for u in ma._path.vertices:
+        for u in pa._path.vertices:
             x, y = list(u)
             points += [(cA * x + cC * y + cE, cB * x + cD * y + cF)]
         cutlist = [points[0: 4]]

To test:

sage: a = arc((0,0),2,1,0,(pi/5,pi/2+pi/12), linestyle="--", color="red")
sage: b = a[0].bezier_path()
sage: c = arc((0,0),2,1,0,(pi/5,pi/2+pi/12))
sage: h = c + b
sage: h

doctest:

sage -t --long src/sage/plot/arc.py
**********************************************************************
File "src/sage/plot/arc.py", line 299, in sage.plot.arc.Arc.bezier_path
Failed example:
    b[0]
Expected:
    Bezier path from (1.618..., 0.5877...) to (-0.5176..., 0.9659...)
Got:
    Bezier path from (1.1338305413635981, 0.82377610785199551) to (-0.26557635219221642, 0.99114443967015675)
**********************************************************************
1 item had failures:
   1 of   7 in sage.plot.arc.Arc.bezier_path
    [47 tests, 1 failure, 6.77 s]
----------------------------------------------------------------------
sage -t --long src/sage/plot/arc.py  # 1 doctest failed
----------------------------------------------------------------------

We do get an arc. The failure is perhaps expected since the original bezier graph was incorrect.

[fbissey]

Could you show the original, so it is clear that the graph was incorrect?

[strogdon]

Replying to fbissey:

Could you show the original, so it is clear that the graph was incorrect?

How to show this? The original will appear almost identical but the limits will be different. Let's try a calculation from Sage. With mpl-1.5.3 and the old code we get

sage: b[0]
Bezier path from (1.6180339887498949, 0.58778525229247314) to (-0.51763809020504215, 0.9659258262890682)

With the revised code using mpl-2.1.0 we get

sage: b[0]
Bezier path from (1.1338305413635981, 0.82377610785199551) to (-0.26557635219221642, 0.99114443967015675)

From the above example, the inputs, theta1 and theta2 are:

sage: theta1 = float(pi)/5; theta1
0.6283185307179586
sage: theta2 = float(pi)/2+float(pi)/12; theta2
1.832595714594046

Then using mpl-1.5.3 b[0] results, the angles are

sage: theta1 = arctan(0.58778525229247314/1.6180339887498949); theta1
0.3484485055191126
theta2 = arctan(0.9659258262890682/-0.51763809020504215) + float(pi); theta2
2.0627485381367054

which are incorrect.

Now using mpl-2.1.0 b[0] results, the angles are

sage: theta1 = arctan(0.82377610785199551/1.1338305413635981); theta1
0.6283185307179587
sage: theta2 = arctan(0.99114443967015675/-0.26557635219221642) + float(pi); theta2
1.8325957145940461

which are correct, to machine accuracy.

[strogdon]

I should have been more careful. The graph isn't correct, is it? The endpoints are not correct. It's because of how things are displayed. Let me think how to correct this?

[strogdon]

The code

sage: a = arc((0,0),2,1,0,(pi/5,pi/2+pi/12), linestyle="--", color="red")
sage: a

displays

While the code

sage: a = arc((0,0),2,1,0,(pi/5,pi/2+pi/12), linestyle="--", color="red")
sage: b = a[0].bezier_path()
sage: b

displays

Previously, I was trying to show that these two graphs are the same.

[fbissey]

I guess it would help if they were on the same scale. The current display makes comparison difficult. I can believe they are the same but same scale would definitely look better.

[strogdon]

Replying to fbissey:

I guess it would help if they were on the same scale. The current display makes comparison difficult. I can believe they are the same but same scale would definitely look better.

Yes, but that will require some investigating.

[strogdon]

sage: a = arc((0,0),2,1,0,(pi/5,pi/2+pi/12), linestyle="--", color="red")
sage: b = a[0].bezier_path()
sage: b.aspect_ratio()
'automatic'
sage: c = arc((0,0),2,1,0,(pi/5,pi/2+pi/12))
sage: c.aspect_ratio()
1.0
sage: c.set_aspect_ratio('automatic')
sage: h = c + b
sage: h

[fbissey]

We'll have to start moving a bit faster. I will include stuff based on the last work by Steve. I will move all versions of sage in sage-on-gentoo to matplotlib 2.1.0 based on this ticket because 1.5.3 has been dropped.

[git]

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

​07b732c	Merge branch 'develop' into matplotlib-2.1.0
​0db7034	Add Steve's corrections to arc.py

[strogdon]

Perhaps ready for review? Or is how to deal with the classic style still up in the air?

[fbissey]

There are still some questions I think, the classic style being one of them. But at this stage I think we should move forward and address new stuff in follow up tickets. There is bound to be some.

[egourgoulhon]

Replying to fbissey:

There are still some questions I think, the classic style being one of them. But at this stage I think we should move forward and address new stuff in follow up tickets. There is bound to be some.

Thanks for the new commits. I agree that it becomes rather urgent to upgrade to Matplotlib 2.1.0. However, somewhat independently of the classic style, there remains the computer modern font issue. As suggested by Steve in comment:129, it suffices to change one line (line 274) in the file local/lib/python2.7/site-packages/matplotlib/mpl-data/matplotlibrc, namely change

mathtext.fontset : dejavusans # Should be 'dejavusans' (default),

to

mathtext.fontset : cm # Should be 'dejavusans' (default),

I guess this is feasible in the post-install phase, isn't it?

Without this, many users who are taking care of plots with nice labels, will be upset. Note that the change of the default to dejavusans generated some debate in the Matplotlib community, see e.g. ​https://github.com/matplotlib/matplotlib/issues/5860/ and ​https://husky.wordpress.com/2017/02/02/revert-matplotlib-2-0-mathtext-default-font-style-to-computer-modern/ .

[egourgoulhon]

Besides (but this may be delayed to a follow up ticket), I am not sure if we shall use the classic style while generating the doc. Actually, this yields many differences with what the user will get in a Sage session, as illustrated in this ​matplotlib page. I understand that using the classic style in sphinx_plot was a workaround to get png images generated with a decent quality (in this respect, compare the two images in the section ​Interpolation of the above page: it is hard to believe that the new default is a progess...). However to restore the png quality, it is not necessary to use the whole classic style, but simply to set the parameter image.interpolation to bilinear, i.e. we could replace the line

plt.style.use('classic')

in src/doc/common/conf.py by

import matplotlib as mpl
mpl.rcParams['image.interpolation'] = 'bilinear'
mpl.rcParams['image.resample'] = False

[fbissey]

There is an option - which is not distro friendly - but could be applied post-install. Replacing the default matplotlibrc by the classic one. Possibly renaming the normal default one to something like default-2.1.0.

[egourgoulhon]

Replying to fbissey:

There is an option - which is not distro friendly - but could be applied post-install. Replacing the default matplotlibrc by the classic one. Possibly renaming the normal default one to something like default-2.1.0.

Note that what I proposed in comment:170 and comment:171 was something weaker: keep the 2.1.0 default style and simply restore the cm fonts for math display in plots. However, what you propose here (i.e. using the classic style in all Sage) sounds reasonable to make a smooth transition for the users. This would also allow us to fix all issues with the new default style (in particular, the figure size issues) and use it after the transition period is over.

[git]

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

​ca9f8e2	make the classic style (from 1.5.3) the default style for now.
​41c21c6	Add a comment on enforcing the classics style in sphinx_plot for sage-on-distro

[fbissey]

I am not 100% happy about the work around but it will do for now. I apply it pre-install because at the end of the day it is far easier to deal with both python 2 and 3. But post-install would be best as the initial template is parsed and adjusted according to options. In fact it could be best to even bring the template back from 1.5.3 (with adjustments for new options and so on). Someone who is not working remotely should check the results.

[egourgoulhon]

Replying to fbissey:

Someone who is not working remotely should check the results.

OK, I'll check this.

[egourgoulhon]

I've checked on a fresh install. Everything is OK, both in the documentation and in Sage sessions (console + Jupyter).

Thank you very much for your work on this! IMHO, we can move on --> positive review.

[vbraun]

sage -t --long --warn-long 78.8 src/sage/plot/arrow.py
**********************************************************************
File "src/sage/plot/arrow.py", line 364, in sage.plot.arrow.Arrow._render_on_subplot
Failed example:
    two_stroke_re.search(contents) is None
Expected:
    True
Got:
    False
**********************************************************************
1 item had failures:
   1 of  18 in sage.plot.arrow.Arrow._render_on_subplot
    [69 tests, 1 failure, 6.16 s]

[fbissey]

OK, that's weird, we specifically patch for this test. @Eric @Dima can you reproduce the error with the branch?

[fbissey]

OK something changed in one of the latest revisions. The matching pattern doesn't work anymore.

[fbissey]

It looks like there are some changes to arrow.py that didn't get committed, that may be the source of the trouble.

[fbissey]

Replying to fbissey:

It looks like there are some changes to arrow.py that didn't get committed, that may be the source of the trouble.

No they are present, I was looking at a single commit rather than the branch.

What did it is the change to "classic style". This is annoying, the dashing stuff depends on the style. That makes it a fragile test in my book. I'd like to revisit it a little bit and document what's going on there a bit better.

[fbissey]

OK, so I am now trying to insert the notion of matplotlib style sheet inside the plotting system and have it default to classic. I have redone the branch from scratch and I haven't corrected arrow.py for the dashing for now. I'd like to be sure I have everything under control. I think the 2D plots are OK but I think I am missing a piece of info about 3D. If someone wants to confirm the stuff lives at u/fbissey/MPL-2.1.0b and I'll switch the ticket to it if other people think it is a good way to deal with our current issues.

[strogdon]

For which 3D items should one look? Perhaps incorrectly, but I have applied the changes in u/fbisey/MPL-2.0b0 on top of this branch and docteting all under src/sage/plot/give no failures. Presumably this has the changes in arrow.py, at least they appear to be there.

[fbissey]

With a pure branch MPL-2.1.0b (I am not sure how you applied that on top of MPL-2.1.0) I expect the arrow.py doctest to be broken (may be not - it depends on whether or not the previous changes were needed because of the new style sheet). I am talking about the quality of the pictures. 2D pictures seems to be OK but I think 3D are a bit small. I suspect I need to apply a style sheet in a different place for 3D plots. I am looking for confirmation of this fact and whether people think it is a good approach.

[egourgoulhon]

Replying to fbissey:

I am looking for confirmation of this fact and whether people think it is a good approach.

I am looking at the new branch this morning. I'll be back in touch soon.

[egourgoulhon]

I've installed MPL-2.1.0 from scratch by pulling the new branch u/fbisey/MPL-2.0b0 (commit d109b7593) in a fresh Sage 8.2.beta2 (just built from a fresh git clone). I confirm that the 3D plots in the doc are of low quality, while the 2D ones are OK. I also confirm that the doctest failure pointed out by Volker in comment:178 is still there (not surprising since in comment:183, you say it is not corrected yet).

[fbissey]

I think I understand now. 2D figures are directly produced by matplotlib but not 3D ones. For 3D an external program like jmol, tachyon or other is used. But for saving while doing the doc they are dumped from the external application to matplotlib. The only place for applying the style is sphinx_plot. Next we'll to work out what's happening with arrow.py.

[egourgoulhon]

Replying to fbissey:

I think I understand now. 2D figures are directly produced by matplotlib but not 3D ones. For 3D an external program like jmol, tachyon or other is used. But for saving while doing the doc they are dumped from the external application to matplotlib. The only place for applying the style is sphinx_plot.

As I suggested in comment:171, maybe it is not necessary to change the style in sphinx_plot but simply insert the lines

import matplotlib as mpl
mpl.rcParams['image.interpolation'] = 'bilinear'
mpl.rcParams['image.resample'] = False

just after

import matplotlib.pyplot as plt

in the code of sphinx_plot.

[dimpase]

has anyone looked into using GR backend of Mathplotlib in Sage: ​https://gr-framework.org/tutorials/matplotlib.html ?

[fbissey]

Replying to dimpase:

has anyone looked into using GR backend of Mathplotlib in Sage: ​https://gr-framework.org/tutorials/matplotlib.html ?

Not here at any rates. Feel free to submit a new optional/experimental package. That reminds me, I have to do something about the optional qhull spkg. Did you end up opening a ticket for it? We need to switch it from the custom basic makefile to cmake to fix your issues with it properly.

[fbissey]

Replying to egourgoulhon:

Replying to fbissey:

I think I understand now. 2D figures are directly produced by matplotlib but not 3D ones. For 3D an external program like jmol, tachyon or other is used. But for saving while doing the doc they are dumped from the external application to matplotlib. The only place for applying the style is sphinx_plot.

As I suggested in comment:171, maybe it is not necessary to change the style in sphinx_plot but simply insert the lines

import matplotlib as mpl
mpl.rcParams['image.interpolation'] = 'bilinear'
mpl.rcParams['image.resample'] = False

just after

import matplotlib.pyplot as plt

in the code of sphinx_plot.

Yes, probably less ham-fisted. Although I'll check the interpolation to match the classic style.

[dimpase]

Replying to fbissey:

That reminds me, I have to do something about the optional qhull spkg. Did you end up opening a ticket for it? We need to switch it from the custom basic makefile to cmake to fix your issues with it properly.

I cannot recall the exact context of this. I do have trouble with scipy+qhull on FreeBSD, is it what you mean, or is it something else?

[strogdon]

I finally got the new branch going. I always forget to make doc-clean && make to make sure the new matplotlib is used everywhere. The 3D are poorer quality and, if I've done things correctly, the generated .pngs are smaller - 512 x 384 compared to 640 x 480. I believe this was mentioned somewhere.

[fbissey]

Replying to dimpase:

Replying to fbissey:

That reminds me, I have to do something about the optional qhull spkg. Did you end up opening a ticket for it? We need to switch it from the custom basic makefile to cmake to fix your issues with it properly.

I cannot recall the exact context of this. I do have trouble with scipy+qhull on FreeBSD, is it what you mean, or is it something else?

The problem is that matplotlib has its own copy of qhull but will happily use an already installed one. But the current spkg doesn't install library with names that matplotlib knows about. It detects qhull presence by headers and then fails to build because it cannot link. It is possible the issue with scipy is similar as scipy also ships its own qhull.

[fbissey]

Switch the active branch on trac. 3D plots should look nice now. Still have to fix arrow.py again. Possibly do qhull as well.

---

New commits:

​3c5d53c	Rebase MPL-2.1.0 upgrade branch
​d109b75	WIP on inserting matplotlib style sheet as a plot option and defaulting it to classic
​9b90efa	Set matplotlib options in sphinx_plot so that 3D plots look nice again.

[strogdon]

I believe now when the arrowhead is drawn with linestyle='dashed' that the following will match

sage: two_stroke_pattern = r'setdash.*stroke.*stroke.*setdash.*setdash'

and it will not match when the arrowhead is draw solid. To draw the arrowhead with linestyle='dashed' I did

src/sage/plot/arrow.py

@@ -437 +437 @@
                             pe1.draw_path(renderer, gc, tpath, affine, rgbFace)
 
             pe1 = ConditionalStroke(CheckNthSubPath(p, 0), [pe.Stroke()])
-            pe2 = ConditionalStroke(CheckNthSubPath(p, 1), [pe.Stroke(dashes={'dash_offset': 0, 'dash_list': None})])
+            pe2 = ConditionalStroke(CheckNthSubPath(p, 1), [pe.Stroke()])
             p.set_path_effects([pe1, pe2])
 
         subplot.add_patch(p)

There may be a better way to do this.

[git]

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

​4005ec3

Fix the arrow matching pattern

[git]

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

​3ea7b16

build qhull with cmake instead of custom script. This install can then be used by matplotlib.

[fbissey]

Bonus, building with optional qhull spkg now works. So one more time in review!

[egourgoulhon]

Replying to fbissey:

Bonus, building with optional qhull spkg now works. So one more time in review!

Thank you Frédéric! The 3D plots in the doc are now of good quality; however they are a little bit too small, which makes the axis labels difficult to read. To recover the same size as in Sage 8.1, it suffices to set some parameters in sphinx_plot. Following ​these suggestions, I've tried

src/doc/common/conf.py

@@ -26 +26 @@
     from sage.misc.temporary_file import tmp_filename
     import matplotlib.pyplot as plt
     import matplotlib as mpl
-    mpl.rcParams['image.interpolation'] = 'bilinear'
-    mpl.rcParams['image.resample'] = False
     if os.environ.get('SAGE_SKIP_PLOT_DIRECTIVE', 'no') != 'yes':
+        mpl.rcParams['image.interpolation'] = 'bilinear'
+        mpl.rcParams['image.resample'] = False
+        mpl.rcParams['figure.figsize'] = [8.0, 6.0]
+        mpl.rcParams['figure.dpi'] = 80
+        mpl.rcParams['savefig.dpi'] = 100
         fn = tmp_filename(ext=".png")
         plot.plot().save(fn)
         img = mpimg.imread(fn)

and it works well: the original (i.e. Sage 8.1) size is restored (side note: all settings of mpl.rcParams are moved to the if block, for there is no need to run them if no plot is to be generated).

[fbissey]

It's François actually, not Frederick. Anyway it starts to fell like setting the style would be shorter but I don't really mind that much at this stage. Incoming.

[egourgoulhon]

Replying to fbissey:

It's François actually, not Frederick.

My apologies about this! I know it is François, but I had just sent an e-mail to a real Frédéric, so I was still in some "Frédéric mode".

[git]

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

​3a6792e

Setting more parameters in sphinx_plot for better 3D documentation

[egourgoulhon]

Thanks for the change in sphinx_plot. The 3D plots in the doc are OK now. Moreover, make ptestlong reveals no error. Hence positive review. Dima, do you agree?

[dimpase]

Replying to fbissey:

Bonus, building with optional qhull spkg now works. So one more time in review!

I think we need a follow-up ticket for the following:

It seems that we need sdh_cmake in addition to sdh_configure etc we already have in src/bin/sage-dist-helpers. This would make spkg-install scripts where cmake is used more uniform and less verbose.

And spkg-install ought to use already available sdh_* things, too. E.g. after cmake call one would only need something like

sdh_make
sdh_make_install

[dimpase]

Replying to egourgoulhon:

Thanks for the change in sphinx_plot. The 3D plots in the doc are OK now. Moreover, make ptestlong reveals no error. Hence positive review. Dima, do you agree?

Do we want an OSX test?

[fbissey]

Replying to dimpase:

Replying to fbissey:

Bonus, building with optional qhull spkg now works. So one more time in review!

I think we need a follow-up ticket for the following:

It seems that we need sdh_cmake in addition to sdh_configure etc we already have in src/bin/sage-dist-helpers. This would make spkg-install scripts where cmake is used more uniform and less verbose.

Sure, but I think we'll only get traction on that when cmake is standard. But I thought strongly about it too.

And spkg-install ought to use already available sdh_* things, too. E.g. after cmake call one would only need something like

sdh_make
sdh_make_install

I should have thought of that. I'll just tweak it.

[fbissey]

Replying to dimpase:

Replying to egourgoulhon:

Thanks for the change in sphinx_plot. The 3D plots in the doc are OK now. Moreover, make ptestlong reveals no error. Hence positive review. Dima, do you agree?

Do we want an OSX test?

Yes please. I am not expecting any bad things but I like some insurance.

[git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

​929509e

use helper scripts in qhull

[dimpase]

probably something for another follow-up ticket. Running headless on OSX 10.13, atop of the clang branch (from the epic clang ticket), I get

sage -t --long --warn-long 476.0 src/sage/plot/plot.py
**********************************************************************
File "src/sage/plot/plot.py", line 464, in sage.plot.plot
Failed example:
    P = plt.plot(t, s, linewidth=1.0)
Expected nothing
Got:
    _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.

which looks like a regression to me.

It can be reproduced at Sage prompt.

[fbissey]

It feels like on OS X matplotlib may want to default the renderer to something OS X specific. I am assuming you have a batch of those, not just the one? This is similar to stuff solved by this patch ​https://anonscm.debian.org/cgit/debian-science/packages/sagemath.git/tree/debian/patches/u1-fix-dont-require-DISPLAY.patch in debian and I think there was ticket on trac about DISPLAY recently.

[fbissey]

Is SAGE_MATPLOTLIB_GUI set to no when you build headless? If not you should.

[dimpase]

Replying to fbissey:

It feels like on OS X matplotlib may want to default the renderer to something OS X specific. I am assuming you have a batch of those, not just the one?

no, it happens just once; while I test src/sage/plot I get just one error. At the Sage prompt I get

sage: import pylab as plt
sage: t = plt.arange(0.0, 2.0, 0.01)
sage: s = sin(2*pi*t)
sage: P = plt.plot(t, s)
_RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.

It's some kind of initialisation error: if I re-run the last line then no 2nd error message is shown.

Note that it is indeed similar to this:

This is similar to stuff solved by this patch ​https://anonscm.debian.org/cgit/debian-science/packages/sagemath.git/tree/debian/patches/u1-fix-dont-require-DISPLAY.patch in debian and I think there was ticket on trac about DISPLAY recently.

If I run (in a fresh Sage session)

sage: import matplotlib; matplotlib.use('Agg')
sage: import pylab as plt
sage: t = plt.arange(0.0, 2.0, 0.01)
sage: s = sin(2*pi*t)
sage: P = plt.plot(t, s)

I see no error message.

---

This could be a clang-only issue, I don't know.

[fbissey]

OK, surprising but then debian didn't patch every single call either. It would be nice to figure the trigger for a window.

[dimpase]

It comes somewhere from the dark depths of matplotlib, this error message. Perhaps just add

import matplotlib; matplotlib.use('Agg')

right before this doctest and be done with?

[fbissey]

Replying to dimpase:

It comes somewhere from the dark depths of matplotlib, this error message. Perhaps just add

import matplotlib; matplotlib.use('Agg')

right before this doctest and be done with?

Sure, but in that case, I'll do the debian guys a favor and fix theirs too.

[dimpase]

By the way, what we see here on OSX is pure matplotlib issue. In ./sage --ipython I see this error message just as well.

In [1]: import pylab as plt

In [2]: t = plt.arange(0.0, 2.0, 0.01)

In [3]: plt.plot(t,t)
_RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.
Out[3]: [<matplotlib.lines.Line2D at 0x10d1446d0>]

In [4]: plt.plot(t,t)
Out[4]: [<matplotlib.lines.Line2D at 0x10d188a90>]

[fbissey]

I never doubted it. I am also quite sure that if you build matplotlib with SAGE_MATPLOTLIB_GUI=no you'll be in the clear too. Because that's probably linked to the macosx backend like the debian guy issue wouldn't happen if they weren't building all the possible GUI backends.

[strogdon]

Replying to fbissey:

I never doubted it. I am also quite sure that if you build matplotlib with SAGE_MATPLOTLIB_GUI=no you'll be in the clear too. Because that's probably linked to the macosx backend like the debian guy issue wouldn't happen if they weren't building all the possible GUI backends.

With matplotlib-1.5.1 the default Sage backend was agg. Now with 2.1.0 the default is tkagg. So maybe there is something wrong with tkagg.

[fbissey]

Really my last build shows that in matplotlibrc I have (without doing any special GUI settings)

#### CONFIGURATION BEGINS HERE

# The default backend; one of GTK GTKAgg GTKCairo GTK3Agg GTK3Cairo
# MacOSX Qt4Agg Qt5Agg TkAgg WX WXAgg Agg Cairo GDK PS PDF SVG
# Template.
# You can also deploy your own backend outside of matplotlib by
# referring to the module name (which must be in the PYTHONPATH) as
# 'module://my_backend'.
#
# If you omit this parameter, it will always default to "Agg", which is a
# non-interactive backend.
backend      : agg

Now I bet Dima has macosx instead.

[strogdon]

I didn't think I had done anything special either, but with this branch

sage: import matplotlib
sage: matplotlib.get_backend()
u'TkAgg'

[fbissey]

OK, I have agg. And I don't have tcl/tk installed. I am wondering why I don't have some of the other backends for which you would think I have the dependencies.

[strogdon]

I do have system tcl/tk installed. So maybe it is picking up on that. From the configure

   OPTIONAL BACKEND EXTENSIONS
                    macosx: no  [skipping due to configuration]
                    qt5agg: no  [PySide2 not found; PyQt5 not found]
                    qt4agg: no  [PySide not found; PyQt4 not found]
                   gtk3agg: no  [Requires pygobject to be installed.]
                 gtk3cairo: no  [Requires cairocffi or pycairo to be installed.]
                    gtkagg: no  [skipping due to configuration]
                     tkagg: yes [installing; run-time loading from Python Tcl /
                            Tk]
                     wxagg: no  [skipping due to configuration]
                       gtk: no  [skipping due to configuration]
                       agg: yes [installing]
                     cairo: no  [cairocffi or pycairo not found]
                 windowing: no  [skipping due to configuration]

[fbissey]

It probably does. tkagg is one of the backend set to auto in setup.cfg. It looks like gtk and gtkagg need pygtk. wxagg needs wxpython. Because all that stuff is actually some kind of python bindings it would have to be installed in sage in the first place. I guess some can be installed by calling sage's pip but OS specific and tkagg are the only ones you can really get out of the box.

[strogdon]

So, should agg be the default and let the user change to whatever? I believe this would fix things on OSX.

[fbissey]

I am not sure how things are ordered when several are available, I'll try to find out. However agg should always be the default in non-interactive mode. Dima and debian's problems may actually be subtle upstream bug of selection of the backend.

[strogdon]

Replying to fbissey:

I am not sure how things are ordered when several are available, I'll try to find out. However agg should always be the default in non-interactive mode. Dima and debian's problems may actually be subtle upstream bug of selection of the backend.

I can replicate the debian problem on gentoo by simply unsetting the DISPLAY and doctesting src/sage/plot/plot.py and src/sage/probability/probability_distribution.pyx. I've seen this may times with sage-on-gentoo on Prefix where I forget to forward my DISPLAY when logging into the Prefix. I don't know about OSX, it looks like a different issue. It may be best to either configure Sage to have agg as the default matplotlib backend (which can be done) or to make sure that prior to using pylab or pyplot in a doctest the matplotlib backend is appropriately set.

[dimpase]

another OSX failure of this type is here:

sage -t --long --warn-long 475.8 src/sage/probability/probability_distribution.pyx
**********************************************************************
File "src/sage/probability/probability_distribution.pyx", line 120, in sage.probability.probability_distribution.ProbabilityDistribut
ion.generate_histogram_data
Failed example:
    h, b = X.generate_histogram_data(bins = 10)
Expected nothing
Got:
    _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.

In case, on OSX the default backend is also TkAgg.

IMHO the right place to set it to Agg is in the testsuite settings. Then no doctstring patches would be needed (there patches are not very useful as far as documentation is concerned, as normally speaking one does not want Agg).

[fbissey]

+1 to that. Do you have a clue where that should be set.

[dimpase]

Replying to fbissey:

+1 to that. Do you have a clue where that should be set.

Somewhere in src/sage/doctest/control.py, but where exactly?

Jeroen, could you tell us? We'd like the doctests to run with import matplotlib; matplotlib.use('Agg') executed.

[git]

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

​1158425

Make sure agg backend is set in matplotlib when doctests are run

[fbissey]

After some thinking I came to the conclusion that it would probably be a burden to add the statement to every single doctests. Which is probably why the settings for the doctests are minimal. While I was looking for something else I noticed something neat about jmol and doctesting. I think it can be adapted here so it will only be effective on matplotlib plots.

Have a go with the new commit!

[strogdon]

This seems to throw a bunch of warnings:

./sage -t --long src/sage/plot/plot.py
**********************************************************************
File "src/sage/plot/plot.py", line 92, in sage.plot.plot
Failed example:
    plot(x^2, (x,0,5))
Expected:
    Graphics object consisting of 1 graphics primitive
Got:
    doctest:warning

...

    UserWarning: 
    This call to matplotlib.use() has no effect because the backend has already
    been chosen; matplotlib.use() must be called *before* pylab, matplotlib.pyplot,
    or matplotlib.backends is imported for the first time.

...

    Graphics object consisting of 1 graphics primitive

This was the only failure in plot.py.

[strogdon]

The UserWarning was not complete. This is what was returned

    UserWarning: 
    This call to matplotlib.use() has no effect because the backend has already
    been chosen; matplotlib.use() must be called *before* pylab, matplotlib.pyplot,
    or matplotlib.backends is imported for the first time.
    <BLANKLINE>
    The backend was *originally* set to u'TkAgg' by the following code:

[fbissey]

Hum... Can you move the code in the commit up a few lines so it is above the other code I introduced to check stylesheet and try again? That block contains an import to matplotlib.pyplot which could be responsible for the message.

[strogdon]

Yes, good spot, that gets rid of the warnings. Unfortunately I'm unable to test the OSX failures.

[git]

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

​8dcadff

Make sure agg backend is set in matplotlib when doctests are run and put the call in the right spot

[fbissey]

I have now redone the commit in the right spot. So Dima should be able to give it a go. Thanks for the testing (that I should have done).

[dimpase]

Good, src/sage/plot/plot.py now passes the tests on OSX, but I still see

**********************************************************************
File "src/sage/probability/probability_distribution.pyx", line 120, in sage.probability.probability_distribution.ProbabilityDistribution.generate_histogram_data
Failed example:
    h, b = X.generate_histogram_data(bins = 10)
Expected nothing
Got:
    _RegisterApplication(), FAILED TO establish the default connection to the WindowServer, _CGSDefaultConnection() is NULL.

I suppose you need to put your agg setting even higher. Otherwise this setting has be done per file, which is kind of silly, IMHO.

[dimpase]

If I do

--- a/src/sage/doctest/control.py
+++ b/src/sage/doctest/control.py
@@ -42,6 +42,8 @@ optionaltag_regex = re.compile(r'^\w+$')
 # Optional tags which are always automatically added
 auto_optional_tags = set(['py2' if six.PY2 else 'py3'])
 
+import matplotlib
+matplotlib.use('agg')
 
 class DocTestDefaults(SageObject):
     """

then all the tests work.

---

Needless to say, this does not affect normal running of Sage, as doctesting is not imported, unless you actually do doctesting.

[fbissey]

Right, I had a look and several functions of probability_distribution.pyx starts with import pylab which would take effect before my own call. I guess there is nothing for it. Let's put it in control.py but I think there is an issue that should be addressed in matplotlib if we could figure a simple test case not involving sage to open an issue.

[dimpase]

one way or another, "real" Sage library code should not be polluted by checks of whether is is being doctested.

[fbissey]

Replying to dimpase:

one way or another, "real" Sage library code should not be polluted by checks of whether is is being doctested.

A laudable goal but not always reachable it seems ​https://github.com/sagemath/sage/blob/master/src/sage/structure/graphics_file.py#L175

I'll be getting code in the branch shortly (first day back to work).

[git]

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

​d7db6e1	Merge branch 'develop' into MPL-2.1.0b
​e0534a8	Make all doctests set agg as the backend for matplotlib.

[dimpase]

the last commit leads to two test failures in src/sage/tests/startup.py - as the import statement in the last commit makes both

    sage: 'numpy' in sys.modules
    False
    sage: 'pyparsing' in sys.modules
    False

fail. I don't know the purpose of these examples - just showing the functionality? We can change numpy to, say, sympy, and pyparsing to something else, too...

[fbissey]

You should read the whole section

Ensure that certain modules are not loaded on startup.
EXAMPLES::
    sage: 'numpy' in sys.modules
    False
    sage: 'pyparsing' in sys.modules
    False
    sage: 'sage.libs.gap.libgap' in sys.modules
    False
Check that IPython is not imported at startup (:trac:`18726`). It is
imported by the doctest framework, so the simple test like above would
not work. Instead, we test this by starting a new Python process::
    sage: from sage.tests.cmdline import test_executable
    sage: cmd = "from sage.all import *\nprint('IPython' in sys.modules)\n"
    sage: print(test_executable(["sage", "--python"], cmd)[0])  # long time
    False

basically we check that numpy and pyparsing are not imported at startup because it slows down the startup process considerably. Because of the manipulation of the last commit the tests would need to be made similar to the one for ipython.

I am feeling like I am playing a game of "whack a mole". I may abandon the most general way to just care for the particular as it is clearly less exhausting.

[strogdon]

Replying to fbissey:

I am feeling like I am playing a game of "whack a mole". I may abandon the most general way to just care for the particular as it is clearly less exhausting.

I agree with this. There is clearly something awry with OSX that maybe should be on another ticket as long as the specific case here is addressed.

[dimpase]

Replying to strogdon:

Replying to fbissey:

I am feeling like I am playing a game of "whack a mole". I may abandon the most general way to just care for the particular as it is clearly less exhausting.

I agree with this. There is clearly something awry with OSX that maybe should be on another ticket as long as the specific case here is addressed.

Well, the change I am proposing should actually as well take care of the problem that Debian people are patching, no? And this is actually a noninvasive way; otherwise Sage starts looking a bit like that famous VW dieselgate code (for running their diesel engines in test mode differently) should have looked...

[fbissey]

Testing something different in test/startup.py? That misses the point the test was introduced for. ​https://github.com/sagemath/sage/commit/bff6915f037ef1d2acbda426f96cb6060509901c last file on that commit.

[dimpase]

Replying to fbissey:

Testing something different in test/startup.py? That misses the point the test was introduced for. ​https://github.com/sagemath/sage/commit/bff6915f037ef1d2acbda426f96cb6060509901c last file on that commit.

I am fine with doing tests for numpy and pyparsing the IPython way, as you proposed. One example how to do this in the normal setting (with libgap) looks sufficient.

[git]

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

​d5be931

Change the startup test for numpy and pyparsing now that they are imported in the doctesting framework via matplotlib.

[fbissey]

And I hope it is the last of it.

[jhpalmieri]

Why not use a test like

    sage: cmd = "print('numpy' in sys.modules)"
    sage: print(test_executable(["sage", "-c", cmd])[0])  # long time
    False

Doesn't that test this issue more directly? On my computer, this one might be a little faster, too.

[jhpalmieri]

(I think the current test works, so we can leave it as is, and consider changes on another ticket.)

[fbissey]

Replying to jhpalmieri:

Why not use a test like

    sage: cmd = "print('numpy' in sys.modules)"
    sage: print(test_executable(["sage", "-c", cmd])[0])  # long time
    False

Doesn't that test this issue more directly? On my computer, this one might be a little faster, too.

Now you are making me think that the ipython test that I copied is actually wrong. sage --python should start a python shell not sage. Correct?

[fbissey]

No, it is correct but yes something more direct looks possible. I should look at the ticket to see why this particular form has been chosen.

[dimpase]

Replying to fbissey:

Replying to jhpalmieri:

Why not use a test like

    sage: cmd = "print('numpy' in sys.modules)"
    sage: print(test_executable(["sage", "-c", cmd])[0])  # long time
    False

Doesn't that test this issue more directly? On my computer, this one might be a little faster, too.

Now you are making me think that the ipython test that I copied is actually wrong. sage --python should start a python shell not sage. Correct?

This is correct, and in fact starting anything short of full Sage is Dieselgate approach to testing...

[git]

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

​a5edbfe	Change the startup test for numpy and pyparsing now that they are imported in the doctesting framework via matplotlib.
​ba7f0ab	Merge branch 'develop' into MPL-2.1.0b

[fbissey]

OK changed to sage -c by popular demand. Rebased on top of 8.2.beta4. Should we have a new positive review now!

[egourgoulhon]

Replying to fbissey:

OK changed to sage -c by popular demand. Rebased on top of 8.2.beta4. Should we have a new positive review now!

LGTM (I've just checked it on Ubuntu, not on MacOS). Thanks again for all your work! (this is comment number 261, pfff!...).

[dimpase]

OK, looks good on OSX too.

[egourgoulhon]

Sorry for removing the positive_review status, but I've just noticed the following doctest errors:

./sage -t --long --warn-long 46.0 src/sage/tests/startup.py
Running doctests with ID 2018-01-31-14-05-27-cae74607.
Git branch: MPL2.1.0
Using --optional=mpir,python2,sage
Doctesting 1 file.
sage -t --long --warn-long 46.0 src/sage/tests/startup.py
**********************************************************************
File "src/sage/tests/startup.py", line 25, in sage.tests.startup
Failed example:
    print(test_executable(["sage", "-c"], cmd)[0])  # long time
Expected:
    False
Got:
    <BLANKLINE>
**********************************************************************
File "src/sage/tests/startup.py", line 28, in sage.tests.startup
Failed example:
    print(test_executable(["sage", "-c"], cmd)[0])  # long time
Expected:
    False
Got:
    <BLANKLINE>
**********************************************************************
1 item had failures:
   2 of  10 in sage.tests.startup
    [9 tests, 2 failures, 4.22 s]
----------------------------------------------------------------------
sage -t --long --warn-long 46.0 src/sage/tests/startup.py  # 2 doctests failed
----------------------------------------------------------------------

This is with Ubuntu 16.04.

[egourgoulhon]

In a Sage console, I indeed get a blank line for the same command:

sage: from sage.tests.cmdline import test_executable
sage: cmd = "print('numpy' in sys.modules)\n"
sage: print(test_executable(["sage", "-c"], cmd)[0])

sage: test_executable(["sage", "-c"], cmd)
('', '', 0)

while I do have:

sage: 'numpy' in sys.modules
False

In the Linux terminal, ./sage -c "print('numpy' in sys.modules)" returns False.

[jhpalmieri]

Please try the test the way I suggested in comment:254 (["sage", "-c", cmd] all in one set of square brackets). Does that work?

[egourgoulhon]

Replying to jhpalmieri:

Please try the test the way I suggested in comment:254 (["sage", "-c", cmd] all in one set of square brackets). Does that work?

Yes this one works:

sage: from sage.tests.cmdline import test_executable
sage: cmd = "print('numpy' in sys.modules)\n"
sage: print(test_executable(["sage", "-c", cmd])[0])
False

sage: test_executable(["sage", "-c", cmd])
('False\n', '', 0)

[fbissey]

Sorry I didn't pay enough attention. I'll redo this ASAP.

[git]

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

​f65ac29	Merge with develop
​ca3b242	correct test in startup.py

[fbissey]

OK, it is corrected.

[dimpase]

OK, I've run the tests on the newest branch now. Apologies for a mis-review the previous time - I only checked files that didn't work before...

[egourgoulhon]

It's also OK from my side: make ptestlong ==> "All tests passed!"

Many thanks!

[jdemeyer]

See #24670 for a follow-up.

[jdemeyer]

Serious breakage at #24671.

[jdemeyer]

Can somebody explain these changes?

@@ -141,7 +140,7 @@ def list_plot3d(v, interpolation_type='default', texture="automatic", point_list
         sage: for i in range(-5, 5):
         ....:     for j in range(-5, 5):
         ....:         l.append((normalvariate(0, 1), normalvariate(0, 1), normalvariate(0, 1)))
-        sage: list_plot3d(l, interpolation_type='nn', texture='yellow', num_points=100)
+        sage: list_plot3d(l, interpolation_type='clough', texture='yellow', num_points=100)
         Graphics3d Object
 
     TESTS:

I'm asking because of this:

sage -t --long --warn-long 10.0 src/sage/plot/plot3d/list_plot3d.py
**********************************************************************
File "src/sage/plot/plot3d/list_plot3d.py", line 143, in sage.plot.plot3d.list_plot3d.list_plot3d
Warning, slow doctest:
    list_plot3d(l, interpolation_type='clough', texture='yellow', num_points=100)
Test ran for 926.47 s
**********************************************************************

926 seconds for a plot which is not even marked # long time?

[fbissey]

Clearly an oversight on my part. interpolation_type='nn' is gone - and not coming back anytime soon. This was discussed at length in this ticket and I pinged sage-devel. Following deafening silence on the options available, I choose a new alternative that looked promising time wise on the example that I timed. As you just shown I haven't timed every single doctests otherwise I may have noticed that this one was putting the new default under stress.

That warrants a follow up ticket to deal with this.

[jdemeyer]

Replying to fbissey:

Clearly an oversight on my part. interpolation_type='nn' is gone - and not coming back anytime soon. This was discussed at length in this ticket and I pinged sage-devel. Following deafening silence on the options available, I choose a new alternative that looked promising time wise on the example that I timed. As you just shown I haven't timed every single doctests otherwise I may have noticed that this one was putting the new default under stress.

I'm not blaiming you... I didn't follow this ticket but given the number of comments, I guess it was not easy.

[jdemeyer]

In 8.2.beta3, the same test took about 3 seconds on the same machine. So we have a regression by a factor 300.

[fbissey]

OK, let's move on to do something about it. Is there any other plots that are taking an awful long time?

[jdemeyer]

Follow-up at #24862.

[jdemeyer]

Welcome to the past: we just got hit by #13135 again.

[gh-dimpase]

By the way, matplotlib 2.2.0 has been released.

[fbissey]

Replying to gh-dimpase:

By the way, matplotlib 2.2.0 has been released.

Is it python-3 only? I remember reading they were getting ready for that at the next "major" release.

[dimpase]

I guess major should be vers. 3

[mmezzarobba]

Probably caused by commit d109b7593ef from this ticket: #25165.