Skip to content

Colorbar only tut #8600

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

Merged
merged 9 commits into from
May 29, 2017
Merged

Conversation

patniharshit
Copy link
Contributor

@patniharshit patniharshit commented May 9, 2017

PR Summary

Trying to convert colorbar_only example to tutorial.

cc @choldgraf

#8561

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/whats_new.rst if major new feature
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

Copy link
Member

@story645 story645 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also cc @choldgraf since he's been kinda lead on documentation lately.

Customized Colorbars Tutorial
=============================

This tutorial shows how to build colorbars without an attached mappable.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a tutorial, use something less jargony than mappable. Without an attached plot maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mappable replaced by plot.

import matplotlib.pyplot as plt
import matplotlib as mpl

fig = plt.figure(figsize=(8, 3))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if you can get away with this since axes stuff isn't the point of this tutorial:
fig, (ax1, ax2, ax3) = plt.subplots(nrows=3, figsize=(8,3))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even figsize is not necessary so removed that too.

# ---------------------------
#
# The second example illustrates the use of a ListedColormap which generates
# colormap from a set of listed colors, a BoundaryNorm which generates a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generates a

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

#
# The second example illustrates the use of a ListedColormap which generates
# colormap from a set of listed colors, a BoundaryNorm which generates a
# colormap index based on discrete interval and extended ends to show the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discrete intervals

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

# Colorbar with custom extension lengths
# --------------------------------------
#
# Now in the third example we illustrate the use of custom length colorbar
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we illustrate the use of custom length colorbar extensions. (They're used on both discrete and continuous colorbars.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am guessing here that 'Now in the third example' is needed to be replaced by 'Here'. Correct me if I am wrong.

# Now in the third example we illustrate the use of custom length colorbar
# extensions, used on a colorbar with discrete intervals. Here we pass colors
# as RGB triplet. To make the length of each extension the same as the length
# of the interior colors pass extendfrac argument as auto
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass the extendfrac argument

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

# ====================
#
# `matplotlib.colorbar.ColorbarBase` derives from `ScalarMappable` and puts a
# colorbar in specified axes, it is the base class with standalone colorbar
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the original is clearer:

ColorbarBase derives from ScalarMappable and puts a colorbar in a specified axes, so it has everything needed for a standalone colorbar.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

#
# `matplotlib.colorbar.ColorbarBase` derives from `ScalarMappable` and puts a
# colorbar in specified axes, it is the base class with standalone colorbar
# drawing functionality. It can be used as-is to make a colorbar for a given
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no dash in as is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dash removed.

# the length of each extension the same as the length of the interior colors
# pass the extendfrac argument as auto.

cmap = mpl.colors.ListedColormap([[0., .4, 1.], [0., .8, 1.],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing color in RGB format, I feel it will be more clear if colors are referenced by name?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the name of the color with RGB = (0, 0.4, 1)?


"""

###############################################################################
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a big deal but you technically don't need to be commenting out lines here, just extend the """ up until the first bit of code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed so as to have uniform format in all tutorials.

# object like an image. In this tutorial we will explore what can be done with
# standalone colorbar.
#
# We will start by making a figure of desired size and adding thress axes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

three axes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is ScalarMappable something we can link to via :mod:? If so we should do it, it's going to be unclear to most people what this is (if it's important)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

# will be used. Then create the colorbar by calling `ColorbarBase` and
# specify axis, colormap, norm and orientation as parameters. Here we create
# a basic continuous colorbar with ticks and labels. There are many more kwargs
# which can be used to further modify the colorbar.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In these kinds of sentences I like to say something like "for more examples, see ". Something worth thinking of inserting where reasonable!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure where could it point to so gave a link to colorbar api.

# outside of the normalized [0,1] range. Here we pass colors as gray shades as
# a string encoding a float in the 0-1 range.
#
# If a ListedColormap is used, the length of the bounds array must be
Copy link
Contributor

@choldgraf choldgraf May 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

":class:matplotlib.colors.ListedColormap"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and elsewhere in any point that we refer to a specific class / function / module in MPL)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

cb3.set_label('Custom extension lengths, some other units')

plt.tight_layout()
plt.show()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stupid question but is there an extra line at the bottom? I think that's best practices but not sure what MPL does as a standard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing it out. My text editor was inserting a new line on save, fixed now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be a newline at the end. If GitHub has that little red symbol on the last line, it means there isn't one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops - yes I meant we wanted the new line, sorry about the confusion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed now.

@choldgraf
Copy link
Contributor

@patniharshit looking really nice! thanks for putting this together. I've got a couple quick points in there but I think it's looking great.

@dstansby dstansby added this to the 2.1 (next point release) milestone May 12, 2017
# the length of each extension the same as the length of the interior colors
# pass the extendfrac argument as auto.
# colorbar with discrete intervals. To make the length of each extension the
# same as the length of the interior colors pass the *extendfrac* argument as
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"...length of the interior colors, use extendfrac='auto'"

@choldgraf
Copy link
Contributor

small point in there about using double backticks for the kwarg but other than that LGTM

# same as the length of the interior colors pass the *extendfrac* argument as
# *auto*.
# colorbar with discrete intervals. To make the length of each extension same
# as the length of the interior colors, use extendfrac='auto'.
Copy link
Contributor

@choldgraf choldgraf May 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry to be picky, but I meant adding in double backticks around it, so:

``extendfrac='auto'``

so that it will render like: extendfrac='auto' instead of "extendfrac='auto'"

I'd edit the branch if I could but I don't have commit rights :-/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@@ -85,7 +85,7 @@
#
# Here we illustrate the use of custom length colorbar extensions, used on a
# colorbar with discrete intervals. To make the length of each extension same
# as the length of the interior colors, use extendfrac='auto'.
# as the length of the interior colors, use ``extendfrac='auto'``.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually do you think you could make this triple backticks?

...JK, this PR looks good to me :)

@patniharshit
Copy link
Contributor Author

Whats the issue here? Why check is failing?

@choldgraf
Copy link
Contributor

Check out the logs in travis: https://travis-ci.org/matplotlib/matplotlib/jobs/231794760

One drawback of sphinx-gallery is that it collects the figure each time it hits another RST block, so you need to re-do fig, ax = plt.subplots each time you want a new figure to show up in the rST. It's one of those annoying errors that works fine when you run the example on your own, but breaks with sphinx-gallery.

@choldgraf
Copy link
Contributor

Looks like tests are passing. @story645 do you know if there's a build that generates the website for us? E.g., MNE uses circle.ci to generate the documentation as a part of the tests, which is a really nice sanity check to make sure it looks correct at the end of documentation PRs.

@patniharshit
Copy link
Contributor Author

patniharshit commented May 15, 2017

Just removed a line in comments which read like draw a figure and three axes. It lost its value after we switched to making seperate fig and axis in each rst block instead of making three axes initially.

@story645
Copy link
Member

story645 commented May 15, 2017

Sorry @choldgraf dunno. I know at least one of the tests builds docs, but dunno which one it renders. Maybe @tacaswell can answer this question?

@QuLogic
Copy link
Member

QuLogic commented May 15, 2017

There are two builds that run the docs, but unfortunately do not upload them anywhere. Travis doesn't hold artifacts though it can go to S3, but I think no-one remembers the key for it...

@choldgraf
Copy link
Contributor

@QuLogic I've found it really useful for building docs, particularly when building them locally is non-trivial (as it is with MPL). I can ask the MNE dev that set it up for us if they can explain how tough it was, in case it'd be useful for MPL...lemme know!

Otherwise, this PR looks good to me

Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks!

@dstansby dstansby merged commit 8d40dce into matplotlib:master May 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants