Add the missingtab10 andtab20* colormaps to the relevantweb page, that are currently missing as noticed by#8073 . I also added theWistia colormap that also appears to be missing from this documentation.

I did a minor clean up of thegrayscale.py script (removing some outdated comments for example). Furthermore, the last commit is an overhaul of thelightness.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 thecolormaps.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 to2.0.0-doc (See@tacaswell's comment in#8073). I am still not totally friend with git and did not manage to directly target the2.0.0-doc branch 🐑 ...

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

afvincent added Documentation status: needs review labels

Feb 26, 2017

afvincent added this to the2.0.1 (next bug fix release) milestone

Feb 26, 2017

Copy link

ContributorAuthor

afvincent commentedFeb 27, 2017

I am not sure that I read thecodecov 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 reviewed

Mar 1, 2017

View reviewed changes

doc/users/plotting/colormaps/colormaps.py Outdated

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

Copy link

Member

phobsonMar 1, 2017

Choose a reason for hiding this comment

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

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

Copy link

ContributorAuthor

afvincentMar 25, 2017

Choose a reason for hiding this comment

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

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

doc/users/plotting/colormaps/lightness.py Outdated


		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,

Copy link

Member

phobsonMar 1, 2017

Choose a reason for hiding this comment

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

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.

Copy link

ContributorAuthor

afvincentMar 25, 2017

Choose a reason for hiding this comment

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

The new order (and the addition of some previously missing colormaps) in3c29961 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.

doc/users/plotting/colormaps/lightness.py Outdated


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

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

Copy link

Member

phobsonMar 1, 2017

Choose a reason for hiding this comment

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

"par" -> "per" ?

Copy link

ContributorAuthor

afvincentMar 1, 2017

Choose a reason for hiding this comment

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

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

Copy link

ContributorAuthor

afvincentMar 25, 2017

Choose a reason for hiding this comment

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

Fixed!

Copy link

ContributorAuthor

afvincent commentedMar 1, 2017

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

Copy link

Member

NelleV commentedMar 8, 2017

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

Copy link

ContributorAuthor

afvincent commentedMar 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 ;) ).

Copy link

ContributorAuthor

afvincent commentedMar 12, 2017

Removing theneed_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 thinkWistia is missing from the colormaps that I demonstrated in the gallery example.

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 added5 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 theadd_missing_cmaps_to_perception_doc branch fromdd553cf to3c29961Compare

March 25, 2017 11:27

Copy link

ContributorAuthor

afvincent commentedMar 25, 2017

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

I do not really understand why, but I modifieddoc/examples/color/colormaps_reference.rst (which more or less looks like a copy ofdoc/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

Copy link

Member

NelleV commentedMar 25, 2017

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

Copy link

Member

NelleV commentedMar 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 approved these changes

Mar 25, 2017

View reviewed changes

Copy link

Member

NelleV left a comment

Choose a reason for hiding this comment

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

LGTM

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

doc/users/plotting/colormaps/lightness.py

		'Sequential (2)': 6, 'Diverging': 6, 'Qualitative': 4,
		'Miscellaneous': 6}

		# Spacing between the colormaps of a subplot

Copy link

Member

QuLogicMar 25, 2017•
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link

ContributorAuthor

afvincentMar 26, 2017

Choose a reason for hiding this comment

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

I do not understand to which situation you are refering to: looking atblame on GH, thedc anddsub 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.

Copy link

Member

QuLogicMar 26, 2017

Choose a reason for hiding this comment

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

Oh, this comment probably shouldn't be here but on asubplots 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.

Copy link

ContributorAuthor

afvincentMar 26, 2017

Choose a reason for hiding this comment

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

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

Copy link

Member

QuLogicMar 26, 2017

Choose a reason for hiding this comment

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

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

phobson approved these changes

Mar 26, 2017

View reviewed changes

Copy link

ContributorAuthor

afvincent commentedMar 26, 2017•
edited
Loading

Please do not merge yet, I saw a minor typo (l.37 onlightness.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 asax.set_yticks([0, 50, 100]) orax.set_yticks([0, 25, 50, 75, 100]) if one want to keep the script length as short as possible.

Fix minor typo

e442647

dstansby approved these changes

Apr 2, 2017

View reviewed changes

dstansby merged commit50bb88a intomatplotlib: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#8156from afvincent/add_missing_cmaps_to_percepti…

8be411f

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

Copy link

Member

dstansby commentedApr 2, 2017

Backported to2.0.x via.8be411f

Labels

Documentation

5 participants

Movatterモバイル変換

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 commentedFeb 26, 2017• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

afvincent commentedFeb 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 commentedMar 1, 2017

Uh oh!

NelleV commentedMar 8, 2017

Uh oh!

afvincent commentedMar 8, 2017

Uh oh!

afvincent commentedMar 12, 2017

Uh oh!

afvincent commentedMar 25, 2017

Uh oh!

NelleV commentedMar 25, 2017

Uh oh!

NelleV commentedMar 25, 2017

Uh oh!

NelleV left a comment

Choose a reason for hiding this comment

Uh oh!

QuLogicMar 25, 2017• editedLoading 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 commentedMar 26, 2017• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

dstansby commentedApr 2, 2017

Uh oh!

Uh oh!

afvincent commentedFeb 26, 2017•
edited
Loading

QuLogicMar 25, 2017•
edited
Loading

afvincent commentedMar 26, 2017•
edited
Loading