DOC: Add missing cmaps to perception doc (fix for #8073) #8156

afvincent · Feb 26, 2017

Fixes #8073 .

Add the missing tab10 and tab20* colormaps to the relevant web page, that are currently missing as noticed by #8073 . I also added the Wistia colormap that also appears to be missing from this documentation.

I did a minor clean up of the grayscale.py script (removing some outdated comments for example). Furthermore, the last commit is an overhaul of the lightness.py script, aiming at making it a bit more resilient to adding or deleting one colormap map without having to tweak a lot of "magical" constant values determined by hand. I manly tried to factorize some parts when it seemed possible to do some.

The documentation is still building well :). Here is a self-contained archive version of the colormaps.html web page, with the updated plots: Choosing_Colormaps_doc.zip. For example, the figures with the Vega-related and Wistia colormaps:

If this was to be merged, please backport it to 2.0.0-doc (See @tacaswell's comment in #8073). I am still not totally friend with git and did not manage to directly target the 2.0.0-doc branch 🐑 ...

Edit: added a keyword to automatically close the related issue.

afvincent · Feb 27, 2017

I am not sure that I read the codecov logs correctly, but as far as I understand them, the failure is related to changes done by commits that are different from the ones of this PR.

phobson · Mar 1, 2017

doc/users/plotting/colormaps/colormaps.py

-                             'pink', 'spring', 'summer', 'winter']),
+         ('Sequential (2)', ['afmhot', 'autumn', 'bone', 'cool', 'copper',
+                             'gist_heat', 'gray', 'hot', 'pink', 'spring',
+                             'Wistia', 'summer', 'winter']),


Taking a step back, I think we have three subgroups of sequential colormaps:

single color

blended colors

temperature/seasons

I guess I'd really like to see plots for each of these groups.

Opinion: And within each group, we should sort/group as logically as possible, as opposed to alphabetically

See 3c29961: I tried to group the colormap in a more logical or hue-base fashion, rather than per alphabetical order.

phobson · Mar 1, 2017

doc/users/plotting/colormaps/lightness.py


 mpl.rcParams.update({'font.size': 12})

-# indices to step through colormap
+# Number of colormap par subplot for particular cmap categories
+_DSUBS = {'Perceptually Uniform Sequential': 4, 'Sequential': 6,


It'd be nice if we could compute the values in someway. I that would require the colors module to be reorganzed in some way. And that's definitely out of the scope of this PR.

The new order (and the addition of some previously missing colormaps) in 3c29961 allow a more uniform set of values for _DSUBS and _DC. Basically only two different values are sufficient to clean reference plots.

I have kept an explicit dictionary with all values inside, in case one would want to further tweak these values. One could also use these dictionary to just describe the non-standard cases and rely on .get(key, default) for the others.

phobson · Mar 1, 2017

doc/users/plotting/colormaps/lightness.py


 mpl.rcParams.update({'font.size': 12})

-# indices to step through colormap
+# Number of colormap par subplot for particular cmap categories


"par" -> "per" ?

Yep (French went through the cracks of my mind…).

afvincent · Mar 1, 2017

@phobson Thank you for the feedback :). I will try to see what is feasible this weekend.

NelleV · Mar 8, 2017

@afvincent don't hesitate to ping us when it is done. This PR disappeared from the frontpage.

afvincent · Mar 8, 2017

@NelleV Ok and don't worry, it is still in my to-do list. I was just (and am still) trying to finish the other stuff I am working on in parallel, like the eventplot stuff (with your comments waiting to be addressed for example ;) ).

afvincent · Mar 12, 2017

Removing the need_review flag for the moment because I still have to see what can be done about @phobson's suggestions, and some other on-going PRs that I have are likely to require more time than I expected in the next few days. I will ping when this PR will be ready again for review.

Note to self: I think Wistia is missing from the colormaps that I demonstrated in the gallery example.

afvincent · Mar 25, 2017

Rebased and added some missing colormaps (binary, gist_gray and gist_yarg). Also tried to group the colormaps in a more logical fashion.

I do not really understand why, but I modified doc/examples/color/colormaps_reference.rst (which more or less looks like a copy of doc/mpl_examples/color/colormaps_reference.py) and it appears to not be tracked by git. Is it expected?

Building the docs results in:
Choosing Colormaps — Matplotlib 2.0.0.post3789.dev0+g0c9a2805f documentation.zip.
The plots for people allergic to zip:
lightness_00.pdf
lightness_01.pdf
lightness_02.pdf
lightness_03.pdf
lightness_04.pdf
lightness_05.pdf
and
grayscale_01_00.pdf
grayscale_01_01.pdf
grayscale_01_02.pdf
grayscale_01_03.pdf
grayscale_01_04.pdf
grayscale_01_05.pdf

Ping to @NelleV and @phobson

NelleV · Mar 25, 2017

the doc/examples folder is generated from the examples. The fact that it is not tracked is to be expected.

NelleV · Mar 25, 2017

Once the sphinx-gallery PR is merged, those python script should really be moved to our examples folder.
I'll do a full review after coffee :D

NelleV

LGTM

QuLogic · Mar 25, 2017

doc/users/plotting/colormaps/lightness.py

+          'Sequential (2)': 6, 'Diverging': 6, 'Qualitative': 4,
+          'Miscellaneous': 6}
+
+# Spacing between the colormaps of a subplot


I think it makes more sense to change the figure size like it used to be done. Now you get a different sized Axes and different number of ticks for each figure.

I do not understand to which situation you are refering to: looking at blame on GH, the dc and dsub strategy is used since at least 3 years. I tried to not modify the spirit of the plots, but rather to refactor what could be, and to make the magic values a bit more uniform across all the different cases.

Oh, this comment probably shouldn't be here but on a subplots line. In any case, I missed the fact that you kept the changing figure size, so it should be the same as before.

But if the figure size changes based on the number of plots, then I don't understand why 00 and 05 have 3 y-ticks, but the others have 5.

I did not even notice the different amount of ticks ^^... If I had to guess, I would say that it may be an artifact from plt.tight_layout() that gives more or less place to the subplots depending on length of the x-tick labels.

That might be it. It would be nice if we didn't have to use tight_layout, but I guess then we'd need to know the size of the colormap labels first.

afvincent · Mar 26, 2017

Please do not merge yet, I saw a minor typo (l.37 on lightness.py). I'll try to fix it as soon as I get back to my laptop.

Edit: the typo has been fixed. I the unconsistent number of y-ticks that @QuLogic noticed is an issue, what about to simply use an ad-hoc formatter? Maybe even something as simple as ax.set_yticks([0, 50, 100]) or ax.set_yticks([0, 25, 50, 75, 100]) if one want to keep the script length as short as possible.

…on_doc DOC: Add missing cmaps to perception doc (fix for #8073)

dstansby · Apr 2, 2017

Backported to 2.0.x via. 8be411f

afvincent added Documentation status: needs review labels Feb 26, 2017

afvincent added this to the 2.0.1 (next bug fix release) milestone Feb 26, 2017

phobson reviewed Mar 1, 2017

View reviewed changes

afvincent removed the status: needs review label Mar 12, 2017

afvincent changed the title ~~DOC: Add missing cmaps to perception doc (fix for #8073)~~ [WiP] DOC: Add missing cmaps to perception doc (fix for #8073) Mar 12, 2017

afvincent added 5 commits March 25, 2017 10:57

Add missing colormaps to the relevant doc pages

c847be2

tweak cmap list order for a better fit of "Wistia"

38526b4

Cleaning up 'grayscale.py' script

a769e5b

Overhaul and refactoring of the 'lightness.py' script

0c9a280

Reordering the colormaps and add a few missing ones

3c29961

afvincent force-pushed the add_missing_cmaps_to_perception_doc branch from dd553cf to 3c29961 Compare March 25, 2017 11:27

NelleV approved these changes Mar 25, 2017

View reviewed changes

NelleV changed the title ~~[WiP] DOC: Add missing cmaps to perception doc (fix for #8073)~~ [MRG+1] DOC: Add missing cmaps to perception doc (fix for #8073) Mar 25, 2017

QuLogic reviewed Mar 25, 2017

View reviewed changes

phobson approved these changes Mar 26, 2017

View reviewed changes

Fix minor typo

e442647

dstansby approved these changes Apr 2, 2017

View reviewed changes

dstansby merged commit 50bb88a into matplotlib:master Apr 2, 2017

dstansby changed the title ~~[MRG+1] DOC: Add missing cmaps to perception doc (fix for #8073)~~ DOC: Add missing cmaps to perception doc (fix for #8073) Apr 2, 2017

dstansby added a commit that referenced this pull request Apr 2, 2017

Merge pull request #8156 from afvincent/add_missing_cmaps_to_percepti…

8be411f

…on_doc DOC: Add missing cmaps to perception doc (fix for #8073)

Search code, repositories, users, issues, pull requests...

Uh oh!

DOC: Add missing cmaps to perception doc (fix for #8073) #8156

DOC: Add missing cmaps to perception doc (fix for #8073) #8156

Uh oh!

Conversation

afvincent commented Feb 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

afvincent commented Feb 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

afvincent commented Mar 1, 2017

Uh oh!

NelleV commented Mar 8, 2017

Uh oh!

afvincent commented Mar 8, 2017

Uh oh!

afvincent commented Mar 12, 2017

Uh oh!

afvincent commented Mar 25, 2017

Uh oh!

NelleV commented Mar 25, 2017

Uh oh!

NelleV commented Mar 25, 2017

Uh oh!

NelleV left a comment

Choose a reason for hiding this comment

Uh oh!

QuLogic Mar 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

afvincent commented Mar 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dstansby commented Apr 2, 2017

Uh oh!

Uh oh!

afvincent commented Feb 26, 2017 •

edited

Loading

QuLogic Mar 25, 2017 •

edited

Loading

afvincent commented Mar 26, 2017 •

edited

Loading