Opened 10 years ago

Closed 9 years ago

#11976 closed enhancement (fixed)

matrix plot colorbars

Reported by: jason Owned by: jason, was
Priority: major Milestone: sage-4.8
Component: graphics Keywords:
Cc: kcrisman Merged in: sage-4.8.alpha5
Authors: Jason Grout Reviewers: Rob Beezer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by rbeezer)

I needed colorbars for matrix plots, so I added them. I also fixed a small bug in how keyword arguments were being passed in contour plot colorbars.

Apply:

  1. trac-11976-matrixcolorbar.patch
  2. trac_11976-colorbar-sizing.patch

Attachments (2)

trac-11976-matrixcolorbar.patch (5.4 KB) - added by jason 10 years ago.
trac_11976-colorbar-sizing.patch (3.6 KB) - added by rbeezer 9 years ago.

Download all attachments as: .zip

Change History (11)

Changed 10 years ago by jason

comment:1 Changed 10 years ago by jason

  • Status changed from new to needs_review

comment:2 Changed 10 years ago by jdemeyer

  • Milestone sage-4.7.3 deleted

Milestone sage-4.7.3 deleted

comment:3 Changed 9 years ago by kcrisman

  • Milestone set to sage-4.8

I like the idea behind this a lot. Using the formatting and colorbar would make doing matrix plots for number theory correctly a snap.

Is it ok if review waits for SD 35.5, though? I just don't know if I'll have time for this, even given the relevance to #9958.

comment:4 Changed 9 years ago by rbeezer

Nice. Passes tests in the obvious directories.

Experimentally, the colorbar would appear to have a length equal to the longest side of the plot. So in one orientation it matches the length of the side, aesthetically pleasing. But in the other orientation it can be grossly larger than the plot. Maybe to make sure the gradations do not get totally smooshed together in the colormap along the short edge?

Anyway, several suggestions:

(a) Document the colorbar defaulting to length of long side.

(b) Use the right combination of max/min on matrix dimensions to provide the right rows:columns ratio to always provide a colorbar the same length as the matrix plot.

(c) Clearly document how to accomplish (b) with an option like colorbar_options={'shrink':0.2} . which seems to be the way to accomplish what I'd like to be easily possible.

(d) (b) and (c)

(e) None of the above.

I could contribute to the docstring if you want to go that route. But it has to be now, not much time for this for many days to come.

Rob

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

I think I prefer (a) and (c), and minimizing the amount of custom code we have on top of matplotlib.

comment:6 in reply to: ↑ 5 Changed 9 years ago by rbeezer

Replying to jason:

I think I prefer (a) and (c), and minimizing the amount of custom code we have on top of matplotlib.

OK, I'll write a documentation-only patch after dinner and we can trade reviews.

Changed 9 years ago by rbeezer

comment:7 Changed 9 years ago by rbeezer

  • Authors set to Jason Grout
  • Description modified (diff)
  • Reviewers set to Rob Beezer

Patch adds a doctest showing how to change the size of a colorbar. The default behavior is not as straightforward as I thought, so it would not have been worth the effort to do this automatically.

There is also some minor cleanup in the documentation, mostly whitespace and line breaks.

With both patches on 4.8.alpha3 this passes all tests and documentation looks fine. I'm clear on a positive review on Jason's patch. If my documentation patch looks OK, go ahead and flip this to positive review.

comment:8 Changed 9 years ago by jason

  • Status changed from needs_review to positive_review

Positive review on Rob's patch. The problem turns out to be creating the extra axis for the colorbar. Horizontally, it spans the full width of the graphic (by default 6 inches, I think). Usually the actual graphic is cut down to the size of the matrix plot, but since the colorbar is there spanning the entire width, it stays at 6 inches.

You showed a good solution, though.

comment:9 Changed 9 years ago by jdemeyer

  • Merged in set to sage-4.8.alpha5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.