Skip to content

Make new colormaps full-fledged citizens - #5283

Merged
WeatherGod merged 4 commits into
matplotlib:masterfrom
mdboom:colormap-fixes
Oct 22, 2015
Merged

WeatherGod merged 4 commits into
matplotlib:masterfrom
mdboom:colormap-fixes

Conversation

@mdboom

@mdboom mdboom commented Oct 20, 2015

Copy link
Copy Markdown
Member

Fix #5202

@mdboom mdboom added this to the next point release (1.5.0) milestone Oct 20, 2015
@WeatherGod

Copy link
Copy Markdown
Member

Not sure if I am happy with the "reduce" name. Makes me think of python's reduce() or numpy's reduce().

@mdboom

mdboom commented Oct 20, 2015

Copy link
Copy Markdown
Member Author

Maybe "segment"?

@WeatherGod

Copy link
Copy Markdown
Member

Eh... better, but not really clear. Maybe a better docstring would help in explaining its purpose. To me, it just looks like a glorified copy constructor...

@mdboom

mdboom commented Oct 20, 2015

Copy link
Copy Markdown
Member Author

It's a copy constructor that changes the number of lookup table entries. Maybe "resample"?

@WeatherGod

Copy link
Copy Markdown
Member

Much better!

@mdboom

mdboom commented Oct 20, 2015

Copy link
Copy Markdown
Member Author

Updated with renamed methods.

@WeatherGod

Copy link
Copy Markdown
Member

by the way... pet peeve of mine is the error message below that prints out the list of keys. The keys aren't sorted, so it changes all the time and it is hard to find the one I want if I misremember the name. Think we can sneak a fix of that here?

@mdboom

mdboom commented Oct 20, 2015

Copy link
Copy Markdown
Member Author

Keys are now sorted in the error message.

@WeatherGod

Copy link
Copy Markdown
Member

+1 from me

@WeatherGod

Copy link
Copy Markdown
Member

do we need a "whats_new"?

@mdboom

mdboom commented Oct 20, 2015

Copy link
Copy Markdown
Member Author

I don't think this needs a what's new -- it's basically just a bugfix that should have been done when the new colormaps were added (and there's already a what's new for those).

@WeatherGod

Copy link
Copy Markdown
Member

I was more referring to the introduction of a new colormap method. Unless you want to make it private for now?

@mdboom

mdboom commented Oct 22, 2015

Copy link
Copy Markdown
Member Author

I'll just make these private for now. The interface to this is really cm.get_cmap (as it's always been).

@WeatherGod

Copy link
Copy Markdown
Member

alright, so it is now +1 from me. Was this intended to get backported to v1.5.x?

@jenshnielsen

Copy link
Copy Markdown
Member

@WeatherGod yes I think so #5202 is tagged at 1.5.0

WeatherGod added a commit that referenced this pull request Oct 22, 2015
Make new colormaps full-fledged citizens
@WeatherGod
WeatherGod merged commit b6d18bb into matplotlib:master Oct 22, 2015
WeatherGod added a commit that referenced this pull request Oct 22, 2015
Make new colormaps full-fledged citizens
@WeatherGod

Copy link
Copy Markdown
Member

backported to v1.5.x as e7e0807

@mdboom
mdboom deleted the colormap-fixes branch November 10, 2015 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants