Opened 11 years ago

Closed 11 years ago

Last modified 10 years ago

#11915 closed enhancement (fixed)

upgrade matplotlib to 1.1.0

Reported by: Jason Grout Owned by: tbd
Priority: major Milestone: sage-5.0
Component: packages: standard Keywords:
Cc: mhampton, François Bissey, Karl-Dieter Crisman Merged in: sage-5.0.beta0
Authors: Jason Grout Reviewers: François Bissey, Jeroen Demeyer, Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11976 Stopgaps:

Status badges

Attachments (1)

trac-11915-matplotlib-1.1.0-upgrade.patch (7.4 KB) - added by Jason Grout 11 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 11 years ago by Jason Grout

(ccing mhampton because he's done a lot of work with animations, so he might be interested in the new matplotlib features)

comment:2 Changed 11 years ago by Jason Grout

Noting what things could be enhanced in Sage when this is upgraded:

It would be great if we supported the new animation api too.

comment:3 Changed 11 years ago by Leif Leonhardy

For the record, I have some preliminary MPL 1.0.1.p1 spkg fixing some issues with pkg-config (cf. comments here).

I should perhaps open another ticket for that and make this ticket depend on it.

comment:4 in reply to:  2 Changed 11 years ago by Leif Leonhardy

Replying to jason:

Noting what things could be enhanced in Sage when this is upgraded: [...]

OTOH, people frequently also ask how to set axis labels, do logarithmic plots, change the font of $something etc.

Right yesterday:

<hiptobecubic> Can i make a 3dplot where the color is determined by the value of the function?
 or any arbitrary function?
 _leif, do YOU know how to specify colors to plot3d?
<hiptobecubic> i can't find any example anywhere of any kind of color functions

(That's just one example. I could only point him to complex_plot(), but I'm not sure whether that's what he was looking for, as it's not really 3D.)

IMHO there should be some tutorial on plotting as well... :P

comment:5 Changed 11 years ago by Jason Grout

Authors: Jason Grout
Cc: François Bissey added
Description: modified (diff)
Status: newneeds_review

fbissey notes that this doctest fails:

sage -t -long -force_lib "devel/sage/sage/plot/colors.py"   
**********************************************************************
File "/usr/share/sage/devel/sage/sage/plot/colors.py", line 1241:
    sage: len(maps.maps)
Expected:
    134
Got:
    138
**********************************************************************
File "/usr/share/sage/devel/sage/sage/plot/colors.py", line 1285:
    sage: len(maps)
Expected:
    134
Got:
    138
**********************************************************************

Also, it seems that at least the marker documentation should be checked (see above note)

comment:6 Changed 11 years ago by Jason Grout

Status: needs_reviewneeds_work

Changed 11 years ago by Jason Grout

comment:7 Changed 11 years ago by Jason Grout

Dependencies: #11976
Status: needs_workneeds_review

comment:8 Changed 11 years ago by Jason Grout

Cc: Karl-Dieter Crisman added

CCing kcrisman, since he'd be a natural one to review this update.

comment:9 in reply to:  8 Changed 11 years ago by Karl-Dieter Crisman

CCing kcrisman, since he'd be a natural one to review this update.

Except for the issue of how the heck to test it on a million different platforms?

I do like the patch. Can you explain why those are the only places you need grid_spec, and what that does? Also,

the figure.tight_layout() function provides prettier graphs.  This also necessitated a change in making colorbar axes

Can you attach a couple before/after on the tight_layout business?

That said, I think our best bet is actually to just have the #9958 package quickly reviewed and then take more time on this, unless you really think the changes here will be trivial. I definitely do not have time for a proper spkg review at this point.

comment:10 Changed 11 years ago by Jason Grout

comment:11 Changed 11 years ago by Jason Grout

Here's an example which is bad before, but good after this patch:

plot(x^2,axes_labels=['x label','y label'],frame=True,fontsize=30,figsize=[4,4],fig_tight=False)

comment:12 in reply to:  10 Changed 11 years ago by Karl-Dieter Crisman

Replying to jason:

See the matplotlib docs for before/after changes: http://matplotlib.sourceforge.net/users/tight_layout_guide.html#plotting-guide-tight-layout, as well as an explanation of the _gridspec change: http://matplotlib.sourceforge.net/users/tight_layout_guide.html#colorbar

"Meanwhile, use of pad at least larger than 0.3 is recommended". Are we doing this?

"This is an experimental feature and may not work for some cases." It looks like this just means that it might stay in the previous way otherwise, but experimental makes me nervous.

As for the example, what's up with fig_tight? I can't find it in our docs. Does this mean it's an undocumented (for us, not them of course) mpl thingie? How likely is it that a user would actually use this (now)?

comment:13 Changed 11 years ago by Jason Grout

fig_tight is "documented" in an example in the save function: http://www.sagemath.org/doc/reference/sage/plot/plot.html#sage.plot.plot.Graphics.save. It's also documented in the show command: http://www.sagemath.org/doc/reference/sage/plot/plot.html#sage.plot.plot.Graphics.show (along with the rest of the show options).

I'm just using the default padding, whatever that is. I think it is 0.15. I'm not sure why there seems to be a difference between that and what the docs say.

If it's controversial, we can remove that one line in the save function.

comment:14 in reply to:  13 Changed 11 years ago by Karl-Dieter Crisman

It's also documented in the show command: http://www.sagemath.org/doc/reference/sage/plot/plot.html#sage.plot.plot.Graphics.show (along with the rest of the show options).

Ah, I was just lazy.

I'm just using the default padding, whatever that is. I think it is 0.15. I'm not sure why there seems to be a difference between that and what the docs say.

If it's controversial, we can remove that one line in the save function.

I just want to make sure it isn't going to do something bad to some plots. Which is why I'd want to spend some real time reviewing it :)

But overall, like most mpl changes, this one seems to open up a limitless trove of graphical wonders to the eyes of Sage mortals.

comment:15 Changed 11 years ago by Jason Grout

Sorry, I was wrong. The default fig_tight is 1.2, which is well above the suggested minimum of 0.3.

comment:16 Changed 11 years ago by Jason Grout

grrr...I mean the default pad in the tight_layout is 1.2.

comment:17 Changed 11 years ago by Jeroen Demeyer

Description: modified (diff)

comment:18 Changed 11 years ago by Jeroen Demeyer

Status: needs_reviewneeds_work

SPKG.txt needs to be updated, I doesn't contain a mention of version 1.1.0, nor a reference to this ticket.

comment:19 Changed 11 years ago by Jason Grout

I was following a trend I've observed in recent spkgs that move those sorts of notes to the hg commit messages (though I didn't reference this ticket in the commit message; maybe I should do that?). Is that trend not okay to follow? I think it makes a lot of sense, and I mention this in the SPKG.txt.

comment:20 in reply to:  19 Changed 11 years ago by Jeroen Demeyer

Replying to jason:

I was following a trend I've observed in recent spkgs that move those sorts of notes to the hg commit messages. Is that trend not okay to follow?

I'm completely unaware of any such trend, this is the first spkg I see doing this. All 128 packages merged since sage-4.7 do have a SPKG.txt entry.

If you want to change this, I think it should be discussed on sage-devel first.

comment:21 Changed 11 years ago by Jason Grout

Status: needs_workneeds_review

Okay, fair enough. I updated the matplotlib-1.1.0.spkg file listed above.

comment:22 Changed 11 years ago by François Bissey

Reviewers: François Bissey
Status: needs_reviewpositive_review

spkg looks good to me. basic testing work patch looks nice. I put a positive review for it. If there is a hidden problem we'll find soon enough but it should be a safe upgrade.

comment:23 Changed 11 years ago by François Bissey

Reviewers: François BisseyFrançois Bissey, Jeroen Demeyer

comment:24 Changed 11 years ago by François Bissey

Reviewers: François Bissey, Jeroen DemeyerFrançois Bissey, Jeroen Demeyer, Karl-Dieter Crisman

comment:25 Changed 11 years ago by Jeroen Demeyer

Milestone: sage-4.8sage-5.0

comment:26 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-5.0.beta0
Resolution: fixed
Status: positive_reviewclosed

comment:27 Changed 10 years ago by Jeroen Demeyer

Adding tight_layout() causes a serious slow-down in plotting: #12854.

Note: See TracTickets for help on using tickets.