adding from_list to custom cmap tutorial #6662

Merged
merged 1 commit into from Jun 29, 2016

Conversation

Projects
None yet
7 participants
Contributor

choldgraf commented Jun 28, 2016

This PR adds a bit more explanation to the docstring for this tutorial, rearranges the data creation to be at the beginning, and adds a small section that shows how to create a colormap from a list of RGB tuples.

@NelleV WDYT?

mdboom added the needs_review label Jun 28, 2016

Contributor

choldgraf commented Jun 28, 2016

Ah, and it creates this extra figure, illustrating the N parameter for from_list: figure_1

Contributor

choldgraf commented Jun 28, 2016

btw, let me know if you want me to change the other objects in this tutorial so they're object-based instead of plt. based. I think somebody mentioned they'd like to see that but don't know if it's worth it or not.

@NelleV NelleV and 2 others commented on an outdated diff Jun 28, 2016

examples/pylab_examples/custom_cmap.py
+colors = [(1, 0, 0), (0, 1, 0), (0, 0, 1)] # R -> G -> B
+n_bins = [2, 5, 10, 100] # Discretizes the interpolation into bins
+cmap_name = 'my_list'
+f, axs = plt.subplots(1, 4, figsize=(10, 5))
@NelleV

NelleV Jun 28, 2016

Contributor

I think the convention is as follow:

fig, axes = plt.subplots()

Maybe @mdboom or @tacaswell can confirm?
In any case, we should be as consistent as possible on the whole file. There are other places in this example that don't use the object oriented interface: do you mind editing those as well (for example l.170 to 182).

@choldgraf

choldgraf Jun 28, 2016

Contributor

Cool - yep I can edit those as well (see my latest comment on the main thread).

@NelleV

NelleV Jun 28, 2016

Contributor

Great! Thanks.

@choldgraf

choldgraf Jun 28, 2016 edited

Contributor

It looks like the colorbar mapping is somewhat different when doing it object-oriented style. If I do im = imshow(... and then call fig.colorbar(im, cax=ax[0, 0]), etc, then it gives a plot like this:

image

I think the colorbars are super tiny in the corners. Know how I can fix this?

@efiring

efiring Jun 28, 2016

Owner

The cax kwarg takes an Axes that you have made yourself to hold the colorbar; I think what you want is the ax kwarg, which takes the parent Axes from which space for the colorbar will be taken.

@choldgraf

choldgraf Jun 28, 2016

Contributor

hooray that worked!

On Tue, Jun 28, 2016 at 10:27 AM, Eric Firing notifications@github.com
wrote:

In examples/pylab_examples/custom_cmap.py
#6662 (comment):

+colors = [(1, 0, 0), (0, 1, 0), (0, 0, 1)] # R -> G -> B
+n_bins = [2, 5, 10, 100] # Discretizes the interpolation into bins
+cmap_name = 'my_list'
+f, axs = plt.subplots(1, 4, figsize=(10, 5))

The cax kwarg takes an Axes that you have made yourself to hold the
colorbar; I think what you want is the ax kwarg, which takes the parent
Axes from which space for the colorbar will be taken.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/matplotlib/matplotlib/pull/6662/files/b380fd7bae75b8dc9d12d24977f177175674f317#r68802996,
or mute the thread
https://github.com/notifications/unsubscribe/ABwSHWIznsZA2Ju7IYpNa1D0U5yXuIR4ks5qQVl6gaJpZM4JAULd
.

@choldgraf

choldgraf Jun 28, 2016

Contributor

(thx)

On Tue, Jun 28, 2016 at 10:29 AM, Chris Holdgraf choldgraf@gmail.com
wrote:

hooray that worked!

On Tue, Jun 28, 2016 at 10:27 AM, Eric Firing notifications@github.com
wrote:

In examples/pylab_examples/custom_cmap.py
#6662 (comment)
:

+colors = [(1, 0, 0), (0, 1, 0), (0, 0, 1)] # R -> G -> B
+n_bins = [2, 5, 10, 100] # Discretizes the interpolation into bins
+cmap_name = 'my_list'
+f, axs = plt.subplots(1, 4, figsize=(10, 5))

The cax kwarg takes an Axes that you have made yourself to hold the
colorbar; I think what you want is the ax kwarg, which takes the parent
Axes from which space for the colorbar will be taken.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/matplotlib/matplotlib/pull/6662/files/b380fd7bae75b8dc9d12d24977f177175674f317#r68802996,
or mute the thread
https://github.com/notifications/unsubscribe/ABwSHWIznsZA2Ju7IYpNa1D0U5yXuIR4ks5qQVl6gaJpZM4JAULd
.

Contributor

NelleV commented Jun 28, 2016

Hi Chris,

Thanks for the patch! I think the additional documentation is great.
Have you tried building the sphinx documentation? If so, do you mind adding an image of how this example page looks on the pull request?

Thanks,
Nelle

@choldgraf choldgraf adding from_list to custom cmap tutorial
963ded8
Contributor

choldgraf commented Jun 28, 2016

I'll try building sphinx after updating the commit. Can you give a quick reminder on how to do this, it's been a while since I've built docs

Contributor

choldgraf commented Jun 28, 2016

Actually it's just python ./doc/make.py html isn't it? I'm getting a weird error about latex_math already being registered. Do you know if there's a way to throw warnings for something like that instead of erroring?

Owner

efiring commented Jun 28, 2016

Use the --allowsphinxwarnings option to make.py.

Contributor

NelleV commented Jun 28, 2016

@efiring this is IMO a bit annoying. If we used the "newer" build system of sphinx, that would not happen. I can easily create a PR with the new Makefile (and make.bat) files. Would that be of any interest to the project?

Owner

jenshnielsen commented Jun 28, 2016

That warning is caused by Sphinx 1.4x being stricter And already fixed on current master. The easiest way to suppress it rather than allowing it is to either rebase on current master or downgrade Sphinx to 1.3x

Owner

jenshnielsen commented Jun 28, 2016

@NelleV Setting warningsaserrors as the default is done very much on purpose and as the person that fixed more than 800 Sphinx warnings before that became possible I would be very hard to convince to change this. Travis builds the docs every a branch is pushed with warnings as errors and it works just fine

Owner

jenshnielsen commented Jun 28, 2016

This is the PR that fixes it #6238 which was merged a few days after Sphinx 1.4 was released

Contributor

NelleV commented Jun 28, 2016

@jenshnielsen I'm running the latest matplotlib, and I still have this problem while using make.py.

Owner

jenshnielsen commented Jun 28, 2016

Sorry I misunderstood I thought you wanted to change to Makefiles to get rid of the warningsaserrors

It's puzzling that you still see this error since travis builds without that warning and has done for a long time

Contributor

NelleV commented Jun 28, 2016

In any case, the make.py file is antiquated. The new version of the matplotlib build would not have required the patch from #6238 . The problem with make.py is that it does not guet updated with the library, but it should. Hence the move from generating the make.py to a makefile that calls itself the sphinx-build executable.

Owner

jenshnielsen commented Jun 28, 2016

Im happy to convert to Makefiles as long as we build the docs with warningsaserrors

Contributor

choldgraf commented Jun 28, 2016

ah thanks for that - sphinx is building now w/ the flag...gonna take a bit of time for it to finish, I'll update when that's done

Contributor

NelleV commented Jun 28, 2016

@jenshnielsen I don't think it makes sense to activate the -W option if we have to disable it to have the documentation build… It is what is happening right now. It doesn't bring any advantages, and has the strong disaventages of increasing the difficulty of building the documentation, and thus of contributing to matplotlib.

Owner

jenshnielsen commented Jun 28, 2016

It at the very least need to be the enabled on Travis as I said I had to fix more than 800 warnings including very significant wrongly rendered pages that went unnoticed for years. Since we made this the default on travis we have caught a large number of pr's with invalid RST syntax which would have been unnoticed and only fixed years later. We can change the default and make this optional again i.e. revert 1af4ca0

Contributor

choldgraf commented Jun 28, 2016

Here's how it looks in the docs:
image

Contributor

NelleV commented Jun 29, 2016

That looks good to me. I don't really understand why the appveyor build fails, and I don't think it has anything to do with your PR.

Thanks for the patch!

Contributor

choldgraf commented Jun 29, 2016

Yeah I couldn't figure that one out either...but didn't seem related. Let
me know if there's anything else you'd like me to do!

Chris

On Tue, Jun 28, 2016 at 10:55 PM, Nelle Varoquaux notifications@github.com
wrote:

That looks good to me. I don't really understand why the appveyor build
fails, and I don't think it has anything to do with your PR.

Thanks for the patch!


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#6662 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABwSHQ6SQaefFHZNYW4pO4noIuo8xyqAks5qQgi7gaJpZM4JAULd
.

NelleV changed the title from adding from_list to custom cmap tutorial to [MRG+1] adding from_list to custom cmap tutorial Jun 29, 2016

Owner

jenshnielsen commented Jun 29, 2016

appveyor have some issues at the moment #6520 is work towards resolving it

@tacaswell tacaswell merged commit 5151cf5 into matplotlib:master Jun 29, 2016

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 70.273%
Details

tacaswell removed the needs_review label Jun 29, 2016

@tacaswell tacaswell added a commit that referenced this pull request Jun 29, 2016

@tacaswell tacaswell Merge pull request #6662 from choldgraf/tutorial_fromlist_cmap
DOC: adding from_list to custom cmap tutorial
9e5b18a
Owner

tacaswell commented Jun 29, 2016

Thanks!

backported to v1.5.x as 9e5b18a

Contributor

choldgraf commented Jun 29, 2016

🎉 🎉 🎉

QuLogic changed the title from [MRG+1] adding from_list to custom cmap tutorial to adding from_list to custom cmap tutorial Oct 16, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment