Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Upgrade matplotlib to 1.2.1 #13693

Closed
jasongrout opened this issue Nov 9, 2012 · 35 comments
Closed

Upgrade matplotlib to 1.2.1 #13693

jasongrout opened this issue Nov 9, 2012 · 35 comments

Comments

@jasongrout
Copy link
Member

Matplotlib 1.2.1 is released now: http://matplotlib.1069221.n5.nabble.com/ANN-matplotlib-1-2-1-release-td40752.html

spkg: http://sage.math.washington.edu/home/jpflori/matplotlib-1.2.1.spkg

apply: attachment: trac_13693-part1.patch

CC: @kcrisman @jpflori

Component: graphics

Author: Jason Grout, John Palmieri,Jean-Pierre Flori

Reviewer: John Palmieri, François Bissey

Merged: sage-5.10.beta0

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

@jasongrout

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:3

I'm trying this out. A few quick comments:

  • In SPKG.txt, the line === matplotlib-1.1.0 (Jason Grout, 09 Nov 2012) === should say 1.2.0.
  • The file patches/mkdir-race-condition.patch should be removed from the repository (according to hg status, the repo is not clean right now).
  • I'm getting doctest failures in the plot directory:
The following tests failed:

	sage -t  devel/sage-main/sage/plot/colors.py # 8 doctests failed
	sage -t  devel/sage-main/sage/plot/graphics.py # 1 doctests failed
	sage -t  devel/sage-main/sage/plot/plot.py # 2 doctests failed
	sage -t  devel/sage-main/sage/plot/plot3d/plot_field3d.py # 1 doctests failed

The ones in colors.py are all of the form

    sage: get_cmap('jet')
Expected:
    <matplotlib.colors.LinearSegmentedColormap instance at 0x...>
Got:
    <matplotlib.colors.LinearSegmentedColormap object at 0x11103e8d0>

and the ones in graphics.py are similar. These are easy to fix, of course. Other failures:

File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/sage-5.4.rc4-IPYTHON/devel/sage-main/sage/plot/plot.py", line 1176:
    sage: len((q1).matplotlib().axes[0].legend().texts) # used to raise AttributeError
Expected:
    1
Got:
    2
**********************************************************************
File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/sage-5.4.rc4-IPYTHON/devel/sage-main/sage/plot/plot.py", line 1186:
    sage: len(p1.matplotlib().axes[0].legend().texts)
Expected:
    1
Got:
    3

Why is this failing now? Was there a change in the format of the legend? The error in plot_field3d.py is ValueError: Colormap red is not recognized. I don't know what to do about this one. I haven't run full doctests, just on the plot directory. I'll let you know if I find anything else.

@jasongrout
Copy link
Member Author

comment:4

Wow, sorry, that was really sloppy on my part. This is definitely needs work right now.

@jasongrout
Copy link
Member Author

comment:5

Those printing issues are weird. I wonder if that's from the upgrade to IPython 0.13.1?

@jasongrout
Copy link
Member Author

comment:6

I've uploaded a new spkg that should fix the two issues with the spkg.

@jhpalmieri
Copy link
Member

comment:7

I think I get the printing issues with both the old and the new IPython. See /scratch/palmieri/sage-5.4.rc4-13693/ on sage.math for a build with a stock 5.4.rc4 plus this spkg.

@dandrake
Copy link
Contributor

dandrake commented Mar 2, 2013

comment:8

I hope we can get this into Sage -- version 1.2 has support for PGF/TikZ output which opens up some pretty amazing opportunities with SageTeX. (Not that I will have time to implement that in the near future, but getting this upgrade in is a necessary first step.)

@kcrisman
Copy link
Member

kcrisman commented Mar 3, 2013

comment:9

See also streamplots - for #10775.

@kiwifb
Copy link
Member

kiwifb commented Mar 3, 2013

comment:10

Definitely would be nice. I had seen the failures before in sage-on-gentoo before forcing 1.1.0. I will see if I can dedicate some time to it this week.

@jhpalmieri
Copy link
Member

comment:11

Regarding the failures: I can easily fix all of them except the ones in plot.py:

        sage: q1 = plot([sin(x), tan(x)], legend_label='trig')
        sage: len(q1.matplotlib().axes[0].legend().texts) # used to raise AttributeError
        1

    ::

    Make sure that we don't get multiple legend labels for plot segments
    (:trac:`11998`)::

        sage: p1 = plot(1/(x^2-1),(x,-2,2),legend_label="foo",detect_poles=True)
        sage: len(p1.matplotlib().axes[0].legend().texts)
        1

For the first one (q1), with the new spkg there are two legend labels ("trig" and "None"), where there used to be just one. For p1, there are now three labels, in direct contradiction of the text. It looks like the picture at #11998, except the labels are "None", "None", and "foo". I don't know how to fix these. See my patch for the other fixes.

@jasongrout

This comment has been minimized.

@jasongrout

This comment has been minimized.

@jpflori
Copy link

jpflori commented Mar 28, 2013

comment:14

I was just doing the same thing this evening to try random Cygwin stuff.

I guess we should look for 1.2.1.

@jpflori jpflori changed the title Upgrade matplotlib to 1.2.0 Upgrade matplotlib to 1.2.1 Mar 28, 2013
@jpflori

This comment has been minimized.

@jpflori
Copy link

jpflori commented Mar 28, 2013

@jpflori
Copy link

jpflori commented Mar 28, 2013

comment:16

Not sure if it's the update wihch changed something but I have no failures in plot.py on my 5.9.beta1 install.

I'm putting this back to needs_review though I could not run "make ptestlong" yet cause the install I have access to is kind of borken because of ATLAS black magic.

@jhpalmieri
Copy link
Member

comment:17

Replying to @jpflori:

Not sure if it's the update wihch changed something but I have no failures in plot.py on my 5.9.beta1 install.

Same for me, too. I'll try make ptestlong on a few machines to see what happens.

@jasongrout
Copy link
Member Author

comment:18

Has anyone manually checked the images connected to the failing doctests from before, just to make sure we're not missing a problem?

@jpflori
Copy link

jpflori commented Mar 28, 2013

comment:19

I had a look at the q1 one to spot the problem but it looked fine (blue sine and other curve, axes, little box with a blue segment and trig, no None anywhere) and was confused and then indeed there was no failing doctest.

@jhpalmieri
Copy link
Member

comment:20

I also looked at the examples by hand, and they looked fine (as opposed to with the 1.2.0 spkg). make ptestlong passed on two platforms: boxen.math.washington.edu and an OS X 10.8.3 machine.

So I'm happy with the spkg. If people are happy with my patch, can we give this a positive review?

@kiwifb
Copy link
Member

kiwifb commented Apr 5, 2013

comment:21

I think the patch is all fine. I had to check if it was the intended behavior for plot_field3d.py (monochrome 3d field, I guess it can be useful sometimes).

@kiwifb
Copy link
Member

kiwifb commented Apr 5, 2013

Changed author from Jason Grout to Jason Grout, John Palmieri,Jean-Pierre Flori

@kiwifb
Copy link
Member

kiwifb commented Apr 5, 2013

Reviewer: John Palmieri, Francois Bissey

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer modified the milestones: sage-5.9, sage-5.10 Apr 5, 2013
@jdemeyer
Copy link

jdemeyer commented Apr 5, 2013

comment:24

I'm assuming the patch should be applied...

@jdemeyer

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Apr 5, 2013

comment:25

Yes I should have done the clean up of the description.

@jdemeyer
Copy link

jdemeyer commented Apr 8, 2013

comment:26

This is very wrong and should be fixed:

        assert(cm is not None) 
    except (TypeError, AssertionError, ValueError): 

AssertionError should never be handled in an exception block, unless you have very good reasons to do so. The obvious solution is replacing assert() by something else.

I know this problem isn't caused by this patch, but since it is easy to fix, you should fix it.

@jdemeyer
Copy link

jdemeyer commented Apr 8, 2013

comment:27

For example, you could do:

try:
    cm = get_cmap(colors)
except (TypeError, ValueError):
    cm = None
if cm is None:
    ...

@jhpalmieri
Copy link
Member

comment:29

Okay, here's a new version of the patch. The only change is this:

diff --git a/sage/plot/plot3d/plot_field3d.py b/sage/plot/plot3d/plot_field3d.py
--- a/sage/plot/plot3d/plot_field3d.py
+++ b/sage/plot/plot3d/plot_field3d.py
@@ -72,8 +72,9 @@
     try:
         from matplotlib.cm import get_cmap
         cm = get_cmap(colors)
-        assert(cm is not None)
-    except (TypeError, AssertionError, ValueError):
+    except (TypeError, ValueError):
+        cm = None
+    if cm is None:
         if isinstance(colors, (list, tuple)):
             from matplotlib.colors import LinearSegmentedColormap
             cm = LinearSegmentedColormap.from_list('mymap',colors)

@jhpalmieri
Copy link
Member

fixes for failures except for plot.py

@jhpalmieri
Copy link
Member

comment:30

Attachment: trac_13693-part1.patch.gz

I'm going to restore the positive review, also; you can view my change as a positive review of Jeroen's suggestion...

@jdemeyer
Copy link

Merged: sage-5.10.beta0

@nthiery
Copy link
Contributor

nthiery commented May 6, 2013

comment:32

As a followup: the new version of matplotlib seems to be causing an issue with saving graphs in pdf. See:

https://groups.google.com/forum/?fromgroups=#!topic/sage-devel/6l9Z4y5rCuM

@jdemeyer
Copy link

jdemeyer commented Jun 6, 2013

Changed reviewer from John Palmieri, Francois Bissey to John Palmieri, François Bissey

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

No branches or pull requests

9 participants