/matplotlib

A method added to Colormap classes to reverse the colormap #6211

Merged
merged 3 commits into from Mar 27, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
  2.1 (next point r...
5 participants
@kjartankg @mdboom @efiring @tacaswell @QuLogic
@kjartankg
Contributor

kjartankg commented Mar 23, 2016

This implements a solution for issue #4271

  • method added to both ListedColormap and LinearSegmentedColormap
  • The parent class Colormap raises NotImplementedError
  • A test was added to test_colors.py

This PR succeeds PR #5899. Following is the initial comment copy/pasted from the previous PR:

A few things that I ran into while implementing this:

  • Functions to reverse Colormap data already existed in matplotlib.cm but they are inconvenient for a user. After seeing that I changed my implementation to resemble them more. Perhaps the old functions should eventually be deprecated?
  • The documentation for LinearSegmentedColormap does not mention that input data can be functions. It is mentioned briefly in the doc for makeMappingArray but should probably be mentioned in the doc for LinearSegmentedColormap.
  • The data is specified as "list of tuples" LinearSegmentedColormap and ListedColormap "a list of matplotlib color specifications, or an equivalent Nx3 or Nx4 floating point array (N rgb or rgba values)". I would suggest that this would be changed to array_like and that the data is then copied into a numpy array for internal use.

figure_1
figure_2
figure_3
figure_4
figure_5
figure_6
@kjartankg kjartankg A method added to Colormap classes to reverse the colormap 
* method added to both ListedColormap and LinearSegmentedColormap
* The parent class Colormap raises NotImplementedError
* A test was added to test_colors.py

All checks have failed

2 failing checks
continuous-integration/appveyor/pr — AppVeyor build failed
Details
continuous-integration/travis-ci/pr — The Travis CI build failed
Details
9ceb6b9

mdboom added the needs_review label Mar 23, 2016

@kjartankg
Contributor

kjartankg commented Mar 23, 2016

Looking at the Tracis CI test, I understand the pep8 failure (missing a blank line in the test_colors.py) but the other failure I don't understand. Can someone explain that to me? It seems to be unrelated to what I've done in this PR.

QuLogic added this to the 2.1 (next point release) milestone Mar 23, 2016

@mdboom
Owner

mdboom commented Mar 23, 2016

The other failure is testing against Numpy nightlies, which is known to be broken. It's an optional test, though -- it doesn't cause the whole thing to fail when it fails.
@mdboom
Owner

mdboom commented Mar 23, 2016

Cc: @efiring
@kjartankg kjartankg Blank line added in test_colors.py for pep8 consistency.

All checks have passed

2 successful checks
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
9f0aa6e
@efiring
Owner

efiring commented Mar 23, 2016

Based on a quick look, I think this looks promising.

@tacaswell tacaswell commented on an outdated diff Mar 26, 2016

View full outdated diff
lib/matplotlib/colors.py
@@ -665,6 +665,30 @@ def _resample(self, lutsize):
"""
raise NotImplementedError()
+ def reversed(self, name=None):
+ """
+ Make a reversed instance of the Colormap.
+
+ NOTE: Function not implemented for base class.
@tacaswell

tacaswell Mar 26, 2016

Owner

Use .. note :: and sphinx will render this better.

@tacaswell tacaswell commented on an outdated diff Mar 26, 2016

View full outdated diff
lib/matplotlib/colors.py
@@ -665,6 +665,30 @@ def _resample(self, lutsize):
"""
raise NotImplementedError()
+ def reversed(self, name=None):
+ """
+ Make a reversed instance of the Colormap.
+
+ NOTE: Function not implemented for base class.
+
+ Parameters
+ ----------
+ name : str, optional
+ The name for the reversed colormap. If it's None the
+ name will be the name of the parent colormap + "_r".
+
+ Raises
@tacaswell

tacaswell Mar 26, 2016

Owner

Don't need this section.
@tacaswell
Owner

tacaswell commented Mar 26, 2016

Can you also add a note in whats_new https://github.com/matplotlib/matplotlib/tree/master/doc/users/whats_new ?
@kjartankg kjartankg Docstring changes made based on comments: 
* NOTE: cnaged to .. note ::
* Raises section removed Colormap.reversed docstring
* methods mentioned in a new file under whats_new

All checks have passed

2 successful checks
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
d2d9971

@tacaswell tacaswell merged commit 6f27f95 into matplotlib:master Mar 27, 2016
2 checks passed

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

tacaswell removed the needs_review label Mar 27, 2016

@tacaswell
Owner

tacaswell commented Mar 27, 2016

@kjartankg Thanks! I think this is your first contribution to mpl, congratulations! Hope we hear from you again.
@kjartankg
Contributor

kjartankg commented Mar 27, 2016

Excellent, thanks! I will most likely have a look at some more things. @tacaswell if there's anything specific that needs to be worked on feel free to point me in that direction. Otherwise I will just browse the issues again.

kjartankg deleted the kjartankg:reverse-cm branch Mar 27, 2016

@tacaswell
Owner

tacaswell commented Mar 27, 2016

Browsing the issues / fixing itches you encounter day-to-day is the best place to start.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment