Implement extend color bar for contourf #8806

ghost · 2017-06-26T15:12:41Z

PR Summary

Implement extend keyword for lognorm color bar on contourf. Travis ci might sometimes timeout during installation (before testing starts) and fails.

PR Checklist

Has Pytest style unit tests
Code is PEP 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/whats_new.rst if major new feature
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

efiring

Thank you for filling in this gap in colorbar.

efiring · 2017-06-27T16:53:03Z

lib/matplotlib/contour.py

+        if self.logscale:
+            tempLocator = LogLocator()
+        if self.locator is not None:
+            tempLocator = self.locator


Instead of using a temporary Locator and then requiring that all Locators have 3 new private methods, I think it would be better to keep everything within the ContourBase class. This will be more readable than using Locator private methods that modify one of their arguments. It will likely require fewer LOC as well.

@efiring Thank you for the input. I took sometimes to think it over more carefully, I will fix it your way.

efiring · 2017-06-27T16:54:03Z

lib/matplotlib/tests/test_colorbar.py

@@ -308,3 +310,34 @@ def test_colorbar_lognorm_extension():
    cb = ColorbarBase(ax, norm=LogNorm(vmin=0.1, vmax=1000.0),
                      orientation='vertical', extend='both')
    assert cb._values[0] >= 0.0
+
+
+@image_comparison(baseline_images=['extended_cbar_with_contourf_min',


Please condense the test down to a single image with 3 subplots, saving time and space.

efiring · 2017-06-27T16:54:14Z

lib/matplotlib/tests/test_colorbar.py

+                                   'extended_cbar_with_contourf_both'],
+                  extensions=['png'])
+def test_extended_colorbar_on_contourf():
+    np.random.seed(1)


efiring · 2017-06-27T16:59:07Z

lib/matplotlib/tests/test_colorbar.py

+    x = np.linspace(-3.0, 3.0, N)
+    y = np.linspace(-2.0, 2.0, N)
+    X, Y = np.meshgrid(x, y)
+    z = (bivariate_normal(X, Y, 0.1, 0.2, 1.0, 1.0)


For the purpose of a test you can use a simpler pattern with far fewer points. For example, you could use

z = np.logspace(0.1, 10, 24).reshape((4, 6))

and omit the X and Y entirely.

efiring · 2017-06-27T17:01:02Z

lib/matplotlib/tests/test_colorbar.py

+    plt.subplot(111)
+    plt.contourf(X, Y, z, cmap=cm.PuBu_r, locator=ticker.LogLocator(),
+                 extend='min')
+    plt.colorbar()


For condensing the example you can use fig, axs = plt.subplots(ncols=3) and then use Axes and Figure method calls.

Thanks. Will fix this test up.

efiring · 2017-06-28T11:31:57Z

Here is what I came up with:

diff --git a/lib/matplotlib/contour.py b/lib/matplotlib/contour.py
index 2b3c21b6d..e72a10c81 100644
--- a/lib/matplotlib/contour.py
+++ b/lib/matplotlib/contour.py
@@ -831,9 +831,6 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
             self.logscale = True
             if norm is None:
                 norm = colors.LogNorm()
-            if self.extend is not 'neither':
-                raise ValueError('extend kwarg does not work yet with log '
-                                 ' scale')
         else:
             self.logscale = False

@@ -1213,10 +1210,19 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
         # (Colorbar needs this even for line contours.)
         self._levels = list(self.levels)

+        if self.logscale:
+            raised = lambda x: x * 1.1
+            lowered = lambda x: x / 1.1
+        else:
+            raised = lambda x: x + 1
+            lowered = lambda x: x - 1
+
         if self.extend in ('both', 'min'):
-            self._levels.insert(0, min(self.levels[0], self.zmin) - 1)
+            lower = lowered(min(self.levels[0], self.zmin))
+            self._levels.insert(0, lower)
         if self.extend in ('both', 'max'):
-            self._levels.append(max(self.levels[-1], self.zmax) + 1)
+            upper = raised(max(self.levels[-1], self.zmax))
+            self._levels.append(upper)
         self._levels = np.asarray(self._levels)

         if not self.filled:
@@ -1228,7 +1234,7 @@ class ContourSet(cm.ScalarMappable, ContourLabeler):
         # ...except that extended layers must be outside the
         # normed range:
         if self.extend in ('both', 'min'):
-            self.layers[0] = -1e150
+            self.layers[0] = 1e-150 if self.logscale else -1e150
         if self.extend in ('both', 'max'):
             self.layers[-1] = 1e150

ghost · 2017-06-28T12:35:54Z

Yes. That can work. But shouldn't self._levels be scaled with whatever base of LogNorm? I.e lamda x: x*some_base ? LogNorm takes more than 1 bases and self._levels is used in calculating rgba. I mean when firing up debugger, I see values of self._levels to be like [10^-6, 10^-5,..., 0.1, 1, 10,100] so I think the extended version should be like [10^-7, 10^-6,..., 0.1, 1, 10,100,1000] and LogNorm can take different bases as well. Hence fixed factor like 1.1 might not work.

efiring · 2017-06-28T13:22:06Z

No, just as the value added or subtracted in the linear case (1) is arbitrary, so long as it is positive, the multiplier or divisor in the log case is also arbitrary, so long as it is greater than 1. It merely needs to push the level outside the normed range so that the "under" and "over" colors of the colormap are used.

ghost · 2017-06-28T13:31:57Z

Ah. I see. Thanks for clarifying that. I think your implementation is better. Please commit it.

tacaswell · 2017-06-29T20:09:25Z

@Tuan333 did you mean to close this and delete your branch?

efiring · 2017-06-29T20:34:48Z

I haven't made an alternative PR yet, but I have committed the implementation suggested above. I find that specifying a LogLocator really doesn't do anything sensible at present, regardless of the extend kwarg. I think I have a fix for that, and to be consistent it will also make an improvement in the case with no locator specified and with the extend kwarg. I think this is a rare use case, so I'm not too worried about the behavior change.

ghost · 2017-06-30T06:44:49Z

@efiring @tacaswell oops... I thought this is not needed anymore and moved on to some other bugs I'm interested in. Should I have left this open? (genuine mistake/question)

efiring · 2017-06-30T10:30:05Z

I think closing it is fine. If you don't see a PR from me on this within a week, ping me.

Tuan333 added 3 commits June 26, 2017 12:54

extending lognorm colorbar to contourf

bc8e4cd

Update ticker.py

105abcc

Update test_colorbar.py

016a6b2

tacaswell added this to the 2.1 (next point release) milestone Jun 26, 2017

tacaswell requested a review from efiring June 26, 2017 18:57

Update tempLocator in case ticker is not given

9fa40cc

efiring requested changes Jun 27, 2017

View reviewed changes

ghost closed this Jun 28, 2017

ghost deleted the new branch June 29, 2017 09:23

efiring mentioned this pull request Jul 3, 2017

WIP: Contour log extension #8834

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement extend color bar for contourf #8806

Implement extend color bar for contourf #8806

ghost commented Jun 26, 2017 •

edited by ghost

Loading

efiring left a comment

efiring Jun 27, 2017

ghost Jun 28, 2017 •

edited by ghost

Loading

efiring Jun 27, 2017

efiring Jun 27, 2017

efiring Jun 27, 2017

efiring Jun 27, 2017

ghost Jun 27, 2017

efiring commented Jun 28, 2017

ghost commented Jun 28, 2017 •

edited by ghost

Loading

efiring commented Jun 28, 2017

ghost commented Jun 28, 2017

tacaswell commented Jun 29, 2017 •

edited

Loading

efiring commented Jun 29, 2017

ghost commented Jun 30, 2017 •

edited by ghost

Loading

efiring commented Jun 30, 2017

Implement extend color bar for contourf #8806

Implement extend color bar for contourf #8806

Conversation

ghost commented Jun 26, 2017 • edited by ghost Loading

PR Summary

PR Checklist

efiring left a comment

Choose a reason for hiding this comment

efiring Jun 27, 2017

Choose a reason for hiding this comment

ghost Jun 28, 2017 • edited by ghost Loading

Choose a reason for hiding this comment

efiring Jun 27, 2017

Choose a reason for hiding this comment

efiring Jun 27, 2017

Choose a reason for hiding this comment

efiring Jun 27, 2017

Choose a reason for hiding this comment

efiring Jun 27, 2017

Choose a reason for hiding this comment

ghost Jun 27, 2017

Choose a reason for hiding this comment

efiring commented Jun 28, 2017

ghost commented Jun 28, 2017 • edited by ghost Loading

efiring commented Jun 28, 2017

ghost commented Jun 28, 2017

tacaswell commented Jun 29, 2017 • edited Loading

efiring commented Jun 29, 2017

ghost commented Jun 30, 2017 • edited by ghost Loading

efiring commented Jun 30, 2017

ghost commented Jun 26, 2017 •

edited by ghost

Loading

ghost Jun 28, 2017 •

edited by ghost

Loading

ghost commented Jun 28, 2017 •

edited by ghost

Loading

tacaswell commented Jun 29, 2017 •

edited

Loading

ghost commented Jun 30, 2017 •

edited by ghost

Loading