Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#11915 closed enhancement (fixed)

upgrade matplotlib to 1.1.0

Reported by: jason Owned by: tbd
Priority: major Milestone: sage-5.0
Component: packages: standard Keywords:
Cc: mhampton, fbissey, kcrisman 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 9 years ago.

Download all attachments as: .zip

Change History (28)

comment:1 Changed 10 years ago by jason

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

comment:2 follow-up: Changed 10 years ago by jason

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 10 years ago by leif

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 10 years ago by leif

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 9 years ago by jason

  • Authors set to Jason Grout
  • Cc fbissey added
  • Description modified (diff)
  • Status changed from new to needs_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 9 years ago by jason

  • Status changed from needs_review to needs_work

Changed 9 years ago by jason

comment:7 Changed 9 years ago by jason

  • Dependencies set to #11976
  • Status changed from needs_work to needs_review

comment:8 follow-up: Changed 9 years ago by jason

  • Cc kcrisman added

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

comment:9 in reply to: ↑ 8 Changed 9 years ago by kcrisman

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 follow-up: Changed 9 years ago by jason

comment:11 Changed 9 years ago by jason

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 9 years ago by kcrisman

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 follow-up: Changed 9 years ago by jason

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 9 years ago by kcrisman

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 9 years ago by jason

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

comment:16 Changed 9 years ago by jason

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

comment:17 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:18 Changed 9 years ago by jdemeyer

  • Status changed from needs_review to needs_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 follow-up: Changed 9 years ago by jason

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 9 years ago by jdemeyer

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 9 years ago by jason

  • Status changed from needs_work to needs_review

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

comment:22 Changed 9 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_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 9 years ago by fbissey

  • Reviewers changed from François Bissey to François Bissey, Jeroen Demeyer

comment:24 Changed 9 years ago by fbissey

  • Reviewers changed from François Bissey, Jeroen Demeyer to François Bissey, Jeroen Demeyer, Karl-Dieter Crisman

comment:25 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-4.8 to sage-5.0

comment:26 Changed 9 years ago by jdemeyer

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

comment:27 Changed 9 years ago by jdemeyer

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

Note: See TracTickets for help on using tickets.