From 577a4872b742a62e09d1e884c26c2408a259e727 Mon Sep 17 00:00:00 2001 From: dcherian Date: Wed, 1 May 2019 21:41:56 -0600 Subject: [PATCH 01/14] plot: If provided with colormap do not modify it. --- doc/whats-new.rst | 5 ++++- xarray/plot/utils.py | 3 ++- xarray/tests/test_plot.py | 5 +++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index 27709a09e7a..c1e6d11a181 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -24,7 +24,10 @@ Enhancements - Character arrays' character dimension name decoding and encoding handled by ``var.encoding['char_dim_name']`` (:issue:`2895`) By `James McCreight `_. - + +- Fix facetgrid colormap bug when ``extend=True``. (:issue:`2932`) + By `Deepak Cherian Date: Wed, 1 May 2019 22:00:37 -0600 Subject: [PATCH 02/14] lint --- xarray/tests/test_plot.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index ba1e66c6b41..0fee8c29de8 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -4,12 +4,10 @@ import numpy as np import pandas as pd import pytest -from numpy.testing import assert_array_equal import xarray as xr import xarray.plot as xplt from xarray import DataArray -from xarray.coding.times import _import_cftime from xarray.plot.plot import _infer_interval_breaks from xarray.plot.utils import ( _build_discrete_cmap, _color_palette, _determine_cmap_params, From c44d8d05751da1157059294aa509db2dcfed9baa Mon Sep 17 00:00:00 2001 From: dcherian Date: Wed, 1 May 2019 22:01:15 -0600 Subject: [PATCH 03/14] pep8 --- xarray/plot/utils.py | 2 +- xarray/tests/test_plot.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/xarray/plot/utils.py b/xarray/plot/utils.py index 3a94868063d..8bca92df098 100644 --- a/xarray/plot/utils.py +++ b/xarray/plot/utils.py @@ -266,7 +266,7 @@ def _determine_cmap_params(plot_data, vmin=None, vmax=None, cmap=None, extend = _determine_extend(calc_data, vmin, vmax) if ((levels is not None or isinstance(norm, mpl.colors.BoundaryNorm)) - and not isinstance(cmap, mpl.colors.Colormap)): + and not isinstance(cmap, mpl.colors.Colormap)): cmap, newnorm = _build_discrete_cmap(cmap, levels, extend, filled) norm = newnorm if norm is None else norm diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 0fee8c29de8..9c1d0a7093a 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -538,7 +538,8 @@ def test_cmap_sequential_option(self): assert cmap_params['cmap'] == 'magma' def test_do_nothing_if_provided_cmap(self): - cmap = mpl.colors.LinearSegmentedColormap.from_list('name', ['r', 'g', 'b']) + cmap = mpl.colors.LinearSegmentedColormap.from_list( + 'name', ['r', 'g', 'b']) cmap_params = _determine_cmap_params(self.data, cmap=cmap) assert cmap_params['cmap'] is cmap From 6fcf4fe72d74c4f5416db297387cfdd30524b404 Mon Sep 17 00:00:00 2001 From: dcherian Date: Thu, 2 May 2019 07:46:15 -0600 Subject: [PATCH 04/14] Fixes. --- xarray/plot/utils.py | 7 ++++++- xarray/tests/test_plot.py | 8 +++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/xarray/plot/utils.py b/xarray/plot/utils.py index 8bca92df098..ecd517c0c58 100644 --- a/xarray/plot/utils.py +++ b/xarray/plot/utils.py @@ -265,8 +265,13 @@ def _determine_cmap_params(plot_data, vmin=None, vmax=None, cmap=None, if extend is None: extend = _determine_extend(calc_data, vmin, vmax) + cmap_is_colormap = ( + isinstance(cmap, mpl.colors.Colormap) + or isinstance(cmap, mpl.colors.LinearSegmentedColormap) + ) + if ((levels is not None or isinstance(norm, mpl.colors.BoundaryNorm)) - and not isinstance(cmap, mpl.colors.Colormap)): + and (not cmap_is_colormap)): cmap, newnorm = _build_discrete_cmap(cmap, levels, extend, filled) norm = newnorm if norm is None else norm diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 9c1d0a7093a..8a4f5b07aa9 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -537,9 +537,11 @@ def test_cmap_sequential_option(self): cmap_params = _determine_cmap_params(self.data) assert cmap_params['cmap'] == 'magma' - def test_do_nothing_if_provided_cmap(self): - cmap = mpl.colors.LinearSegmentedColormap.from_list( - 'name', ['r', 'g', 'b']) + @pytest.mark.parametrize( + 'cmap', [mpl.colors.LinearSegmentedColormap.from_list('name', ['r', 'g', 'b']), + mpl.colors.ListedColormap(['r', 'g', 'b']), + 'RdBu_r']) + def test_do_nothing_if_provided_cmap(self, cmap): cmap_params = _determine_cmap_params(self.data, cmap=cmap) assert cmap_params['cmap'] is cmap From 0b27b70a7380df832cfb318d6fc1c787d1d59f1e Mon Sep 17 00:00:00 2001 From: dcherian Date: Thu, 2 May 2019 07:53:53 -0600 Subject: [PATCH 05/14] More fixes. --- xarray/plot/utils.py | 4 ++-- xarray/tests/test_plot.py | 11 ++++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/xarray/plot/utils.py b/xarray/plot/utils.py index ecd517c0c58..2520eed2a58 100644 --- a/xarray/plot/utils.py +++ b/xarray/plot/utils.py @@ -266,8 +266,8 @@ def _determine_cmap_params(plot_data, vmin=None, vmax=None, cmap=None, extend = _determine_extend(calc_data, vmin, vmax) cmap_is_colormap = ( - isinstance(cmap, mpl.colors.Colormap) - or isinstance(cmap, mpl.colors.LinearSegmentedColormap) + isinstance(cmap, mpl.colors.LinearSegmentedColormap) + or isinstance(cmap, mpl.colors.ListedColormap) ) if ((levels is not None or isinstance(norm, mpl.colors.BoundaryNorm)) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 8a4f5b07aa9..e495bcff967 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -539,12 +539,17 @@ def test_cmap_sequential_option(self): @pytest.mark.parametrize( 'cmap', [mpl.colors.LinearSegmentedColormap.from_list('name', ['r', 'g', 'b']), - mpl.colors.ListedColormap(['r', 'g', 'b']), - 'RdBu_r']) + mpl.colors.ListedColormap(['r', 'g', 'b'])]) def test_do_nothing_if_provided_cmap(self, cmap): - cmap_params = _determine_cmap_params(self.data, cmap=cmap) + cmap_params = _determine_cmap_params(self.data, cmap=cmap, levels=7) assert cmap_params['cmap'] is cmap + def test_do_something_if_provided_str_cmap(self): + cmap = 'RdBu_r' + cmap_params = _determine_cmap_params(self.data, cmap=cmap, levels=7) + assert cmap_params['cmap'] is not cmap + assert isinstance(cmap_params['cmap'], mpl.colors.ListedColormap) + def test_cmap_sequential_explicit_option(self): with xr.set_options(cmap_sequential=mpl.cm.magma): cmap_params = _determine_cmap_params(self.data) From 053d872776bc93b3120f9748a3e216e7b670c841 Mon Sep 17 00:00:00 2001 From: dcherian Date: Thu, 2 May 2019 10:09:29 -0600 Subject: [PATCH 06/14] autopep8 --- xarray/plot/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/plot/utils.py b/xarray/plot/utils.py index 2520eed2a58..c945b17302b 100644 --- a/xarray/plot/utils.py +++ b/xarray/plot/utils.py @@ -271,7 +271,7 @@ def _determine_cmap_params(plot_data, vmin=None, vmax=None, cmap=None, ) if ((levels is not None or isinstance(norm, mpl.colors.BoundaryNorm)) - and (not cmap_is_colormap)): + and (not cmap_is_colormap)): cmap, newnorm = _build_discrete_cmap(cmap, levels, extend, filled) norm = newnorm if norm is None else norm From f59eebaeb2f3db23208f78ad05dbc97179169dc3 Mon Sep 17 00:00:00 2001 From: dcherian Date: Thu, 2 May 2019 10:10:56 -0600 Subject: [PATCH 07/14] more pep8 --- xarray/tests/test_plot.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index e495bcff967..540172c2421 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -538,8 +538,10 @@ def test_cmap_sequential_option(self): assert cmap_params['cmap'] == 'magma' @pytest.mark.parametrize( - 'cmap', [mpl.colors.LinearSegmentedColormap.from_list('name', ['r', 'g', 'b']), - mpl.colors.ListedColormap(['r', 'g', 'b'])]) + 'cmap', [ + mpl.colors.LinearSegmentedColormap.from_list('name', ['r', 'g', 'b']), + mpl.colors.ListedColormap(['r', 'g', 'b']) + ]) def test_do_nothing_if_provided_cmap(self, cmap): cmap_params = _determine_cmap_params(self.data, cmap=cmap, levels=7) assert cmap_params['cmap'] is cmap From 0f00522411ba98e805262787467c72734eca533a Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Thu, 2 May 2019 15:47:47 -0600 Subject: [PATCH 08/14] whats-new under bugfixes --- doc/whats-new.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/doc/whats-new.rst b/doc/whats-new.rst index c1e6d11a181..168dbb7f36c 100644 --- a/doc/whats-new.rst +++ b/doc/whats-new.rst @@ -25,9 +25,6 @@ Enhancements ``var.encoding['char_dim_name']`` (:issue:`2895`) By `James McCreight `_. -- Fix facetgrid colormap bug when ``extend=True``. (:issue:`2932`) - By `Deepak Cherian `_. - Return correct count for scalar datetime64 arrays (:issue:`2770`) By `Dan Nowacki `_. +- Fix facetgrid colormap bug when ``extend=True``. (:issue:`2932`) + By `Deepak Cherian Date: Fri, 3 May 2019 07:28:46 -0600 Subject: [PATCH 09/14] Simpler colormap check. --- xarray/plot/utils.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/xarray/plot/utils.py b/xarray/plot/utils.py index c945b17302b..0a507993cd6 100644 --- a/xarray/plot/utils.py +++ b/xarray/plot/utils.py @@ -265,13 +265,8 @@ def _determine_cmap_params(plot_data, vmin=None, vmax=None, cmap=None, if extend is None: extend = _determine_extend(calc_data, vmin, vmax) - cmap_is_colormap = ( - isinstance(cmap, mpl.colors.LinearSegmentedColormap) - or isinstance(cmap, mpl.colors.ListedColormap) - ) - if ((levels is not None or isinstance(norm, mpl.colors.BoundaryNorm)) - and (not cmap_is_colormap)): + and (not isinstance(cmap, mpl.colors.Colormap))): cmap, newnorm = _build_discrete_cmap(cmap, levels, extend, filled) norm = newnorm if norm is None else norm From c25126ba09594942b6c0d667a2196edfca5bf626 Mon Sep 17 00:00:00 2001 From: dcherian Date: Fri, 3 May 2019 07:29:54 -0600 Subject: [PATCH 10/14] lint. --- xarray/tests/test_plot.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 540172c2421..29201b553ad 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -537,11 +537,10 @@ def test_cmap_sequential_option(self): cmap_params = _determine_cmap_params(self.data) assert cmap_params['cmap'] == 'magma' - @pytest.mark.parametrize( - 'cmap', [ - mpl.colors.LinearSegmentedColormap.from_list('name', ['r', 'g', 'b']), - mpl.colors.ListedColormap(['r', 'g', 'b']) - ]) + @pytest.mark.parametrize('cmap', [ + mpl.colors.LinearSegmentedColormap.from_list('name', ['r', 'g', 'b']), + mpl.colors.ListedColormap(['r', 'g', 'b']) + ]) def test_do_nothing_if_provided_cmap(self, cmap): cmap_params = _determine_cmap_params(self.data, cmap=cmap, levels=7) assert cmap_params['cmap'] is cmap From 30b1c67feea0ae70b5cc5c9fed9375294f80dd3d Mon Sep 17 00:00:00 2001 From: Deepak Cherian Date: Fri, 3 May 2019 09:50:01 -0600 Subject: [PATCH 11/14] @requires_matplotlib why is this needed? --- xarray/tests/test_plot.py | 1 + 1 file changed, 1 insertion(+) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 29201b553ad..d81fca8d352 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -537,6 +537,7 @@ def test_cmap_sequential_option(self): cmap_params = _determine_cmap_params(self.data) assert cmap_params['cmap'] == 'magma' + @requires_matplotlib @pytest.mark.parametrize('cmap', [ mpl.colors.LinearSegmentedColormap.from_list('name', ['r', 'g', 'b']), mpl.colors.ListedColormap(['r', 'g', 'b']) From 5fc17bde99c5755a18aa179fcfb3ce09d93750ec Mon Sep 17 00:00:00 2001 From: dcherian Date: Mon, 6 May 2019 11:56:41 -0600 Subject: [PATCH 12/14] Fix test --- xarray/tests/test_plot.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 29201b553ad..bbec056785a 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -537,13 +537,16 @@ def test_cmap_sequential_option(self): cmap_params = _determine_cmap_params(self.data) assert cmap_params['cmap'] == 'magma' - @pytest.mark.parametrize('cmap', [ - mpl.colors.LinearSegmentedColormap.from_list('name', ['r', 'g', 'b']), - mpl.colors.ListedColormap(['r', 'g', 'b']) - ]) - def test_do_nothing_if_provided_cmap(self, cmap): - cmap_params = _determine_cmap_params(self.data, cmap=cmap, levels=7) - assert cmap_params['cmap'] is cmap + def test_do_nothing_if_provided_cmap(self): + # can't parametrize with mpl objects + for cmap in [ + mpl.colors.LinearSegmentedColormap.from_list( + 'name', ['r', 'g', 'b']), + mpl.colors.ListedColormap(['r', 'g', 'b'])]: + cmap_params = _determine_cmap_params(self.data, + cmap=cmap, + levels=7) + assert cmap_params['cmap'] is cmap def test_do_something_if_provided_str_cmap(self): cmap = 'RdBu_r' From 2fca3b4a344073beb163b222931efaa9c3ca44ff Mon Sep 17 00:00:00 2001 From: dcherian Date: Mon, 6 May 2019 11:59:32 -0600 Subject: [PATCH 13/14] neaten --- xarray/tests/test_plot.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index bbec056785a..93246514c99 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -538,11 +538,13 @@ def test_cmap_sequential_option(self): assert cmap_params['cmap'] == 'magma' def test_do_nothing_if_provided_cmap(self): + cmap_list = [ + mpl.colors.LinearSegmentedColormap.from_list('name', ['r', 'g']), + mpl.colors.ListedColormap(['r', 'g', 'b']) + ] + # can't parametrize with mpl objects - for cmap in [ - mpl.colors.LinearSegmentedColormap.from_list( - 'name', ['r', 'g', 'b']), - mpl.colors.ListedColormap(['r', 'g', 'b'])]: + for cmap in cmap_list: cmap_params = _determine_cmap_params(self.data, cmap=cmap, levels=7) From 0b84373f3ec6836f136f1b09d7d8c47ad01a6f5b Mon Sep 17 00:00:00 2001 From: dcherian Date: Mon, 6 May 2019 12:01:03 -0600 Subject: [PATCH 14/14] update comment. --- xarray/tests/test_plot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xarray/tests/test_plot.py b/xarray/tests/test_plot.py index 93246514c99..3c4485e45d0 100644 --- a/xarray/tests/test_plot.py +++ b/xarray/tests/test_plot.py @@ -543,7 +543,7 @@ def test_do_nothing_if_provided_cmap(self): mpl.colors.ListedColormap(['r', 'g', 'b']) ] - # can't parametrize with mpl objects + # can't parametrize with mpl objects when mpl is absent for cmap in cmap_list: cmap_params = _determine_cmap_params(self.data, cmap=cmap,