Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13693 closed task (fixed)

Upgrade matplotlib to 1.2.1

Reported by: jason Owned by: jason, was
Priority: major Milestone: sage-5.10
Component: graphics Keywords:
Cc: kcrisman, jpflori Merged in: sage-5.10.beta0
Authors: Jason Grout, John Palmieri,Jean-Pierre Flori Reviewers: John Palmieri, François Bissey
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Attachments (1)

trac_13693-part1.patch (4.6 KB) - added by jhpalmieri 8 years ago.
fixes for failures except for plot.py

Download all attachments as: .zip

Change History (34)

comment:1 Changed 8 years ago by jason

  • Description modified (diff)

comment:2 Changed 8 years ago by jason

  • Status changed from new to needs_review

comment:3 Changed 8 years ago by jhpalmieri

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.

comment:4 Changed 8 years ago by jason

  • Status changed from needs_review to needs_work

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

comment:5 Changed 8 years ago by jason

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

comment:6 Changed 8 years ago by jason

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

comment:7 Changed 8 years ago by jhpalmieri

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.

comment:8 Changed 8 years ago by ddrake

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

comment:9 Changed 8 years ago by kcrisman

See also streamplots - for #10775.

comment:10 Changed 8 years ago by fbissey

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.

comment:11 Changed 8 years ago by jhpalmieri

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.

comment:12 Changed 8 years ago by jason

  • Description modified (diff)

comment:13 Changed 8 years ago by jason

  • Description modified (diff)

comment:14 Changed 8 years ago by jpflori

  • Cc jpflori added
  • Summary changed from Upgrade matplotlib to 1.2.0 to Upgrade matplotlib to 1.2.1

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

I guess we should look for 1.2.1.

comment:15 Changed 8 years ago by jpflori

  • Description modified (diff)

comment:16 follow-up: Changed 8 years ago by jpflori

  • Status changed from needs_work to needs_review

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.

comment:17 in reply to: ↑ 16 Changed 8 years ago by jhpalmieri

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.

comment:18 Changed 8 years ago by jason

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

comment:19 Changed 8 years ago by jpflori

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.

comment:20 Changed 8 years ago by jhpalmieri

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?

comment:21 Changed 8 years ago by fbissey

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

comment:22 Changed 8 years ago by fbissey

  • Authors changed from Jason Grout to Jason Grout, John Palmieri,Jean-Pierre Flori
  • Reviewers set to John Palmieri, Francois Bissey
  • Status changed from needs_review to positive_review

comment:23 Changed 8 years ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-5.9 to sage-5.10

comment:24 Changed 8 years ago by jdemeyer

  • Description modified (diff)

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

comment:25 Changed 8 years ago by fbissey

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

comment:26 Changed 8 years ago by jdemeyer

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.

comment:27 Changed 8 years ago by jdemeyer

For example, you could do:

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

comment:28 Changed 8 years ago by jdemeyer

  • Status changed from positive_review to needs_work

comment:29 Changed 8 years ago by jhpalmieri

  • Status changed from needs_work to needs_review

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

  • sage/plot/plot3d/plot_field3d.py

    diff --git a/sage/plot/plot3d/plot_field3d.py b/sage/plot/plot3d/plot_field3d.py
    a b  
    7272    try:
    7373        from matplotlib.cm import get_cmap
    7474        cm = get_cmap(colors)
    75         assert(cm is not None)
    76     except (TypeError, AssertionError, ValueError):
     75    except (TypeError, ValueError):
     76        cm = None
     77    if cm is None:
    7778        if isinstance(colors, (list, tuple)):
    7879            from matplotlib.colors import LinearSegmentedColormap
    7980            cm = LinearSegmentedColormap.from_list('mymap',colors)

Changed 8 years ago by jhpalmieri

fixes for failures except for plot.py

comment:30 Changed 8 years ago by jhpalmieri

  • Status changed from needs_review to positive_review

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

comment:31 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.10.beta0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:32 Changed 8 years ago by nthiery

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

comment:33 Changed 8 years ago by jdemeyer

  • Reviewers changed from John Palmieri, Francois Bissey to John Palmieri, François Bissey
Note: See TracTickets for help on using tickets.