Current coverage is 62.10% (diff: 100%)

Merging #7716 into master will decrease coverage by 4.48%

@@             master      #7716   diff @@
==========================================
  Files           109        174     +65   
  Lines         46648      56062   +9414   
  Methods           0          0           
  Messages          0          0           
  Branches          0          0           
==========================================
+ Hits          31061      34815   +3754   
- Misses        15587      21247   +5660   
  Partials          0          0           

Powered by Codecov. Last update 07f6427...615ca22

👍 , left some comments about the API

This will definitely need a whats_new entry.

Most of the suggested changes have been made. A few things:

1. Does the whats_new entry go into doc/api/api_changes.rst?
2. Any suggestions on where to put the join_colormaps function?
3. I had to remake the image tests when I switched to returning only ListedColormap. We probably don't need the lsc test images anymore. Agree?

After adding the __add__ method, it got me thinking about how to do something similar for truncate. Since these ideas are a bit half-baked, I started a new branch. Thoughts on this?

@tacaswell I'm sure you're busy. I just wanted to make sure this wasn't forgotten. Any thoughts on my questions above?

Can anyone help me understand what is breaking here?

It looks like it's just PEP8 violations, for lines that are longer than 79 characters. You should be able to see which lines are causing the error here: https://travis-ci.org/matplotlib/matplotlib/jobs/197464743#L1767

Not sure if there is bandwidth to include this in 2.1, but I'm available to work on it. I think I've addressed all existing comments, except for the whats_new entry. Where do I put that?

Also, I've addressed @WeatherGod 's comments on the proposed __getitem__ method. I'll plan to merge that into this soon, unless there are objections?

OK. I think this is ready to go. Let me know if there is anything else you need.

At the weekly call this week this got substantial discussion. The upshot was that this seems a lot of maintenence, support, and API overhead for something that the user should be able to do themselves, perhaps with better documentation. Basically truncating and subsetting colormaps boils down to making the appropriate matrix to feed to ListedColormap via whatever tools the user is comfortable using in numpy to achieve that end. The major disadvantage of doing it the proposed way, was felt to be the ambiguity of colormap length, and hence the diffculting of knowing whether the join point of two concatenated colormaps was in the middle, or at 1/3 etc. If people have to explicitly specify the length of the colormap, they are most of the way to just manipulating the arrays directly.

The Todo here is to add a section to https://matplotlib.org/tutorials/colors/colormaps.html#sphx-glr-tutorials-colors-colormaps-py that explains colormap manipulation better, and perhaps adding an example or two to the example directory.

@lkilcher thanks so much for the contribution, and sorry it languished so long. If you want to give input to the examples when those PRs come in, I'll ping you, or contributions along those lines would be most welcome.

This is disappointing, and it's too bad this conversation didn't happen before I put time into the tests etc.. I can understand that __getitem__ is more than the maintainers wanted to handle, but .join and truncate are so necessary! God knows how much time I've spent digging into the code to 're-remember' how to join/truncate colormaps (and it's been changing)! If people are worried about N ambiguity, make that a required argument of the functions.

Making a clearly documented interface to the arrays makes sense, and certainly replaces the need for __getitem__, but I still think cm = cm1 + cm2 -- or at least cm = join_colormaps(cm1, cm2) -- is a lot nicer than cm = ListedColormap(np.vstack(cm1.array, cm2.array)).

I apologize for coming across as a disgruntled contributor. I just know quite a few people who find MPL colormaps very cumbersome, and I put time into this because it got positive feedback from some senior MPL devs. I also know there are those out there who think creating/modifying new colormaps is a bad idea, but I find that reasoning arrogant. Hopefully someone will actually work on MEP21 (especially killing LSCs), and perhaps a simple join_colormaps func (and __add__ method) will make sense at that time.

I'll track of your progress on #11905 and see if I have anything to add.

FWIW I share your concern about the opacity of colormaps and how to create them, hence #11905, because info about ListedColormaps is completely missing from the existent docs. Frankly, I didn't know the class existed before Monday. Given that class, however, we can go back and forth relatively painlessly between np arrays and colormaps, it seemed worth trying to improve the documentation first. If the consensus is still that its still too clunky to manipulate colormaps, then we can revisit the idea of allowing the more holistic solution proposed here.

WRT being able to create your own colormaps, I hope I didn't make it sound like people shouldn't be allowed to do that, or that anyone on the dev team thinks that.

In general, the devs are concerned about the balance between "what we can do" and "what we should do". Some features got out of control in the past, and are now maintenance drags on the library. I think the current philosophy is to try to keep things as lightweight as possible and to have fewer black boxes.

Closing in lieu of documentation of how to do the same thing w/ ListedColormaps. @lkilcher again apologies for the long review process that eventually didn't get merged. We really want people to contribute, so we try to avoid this, but sometimes ideas fall between the cracks.