From 506bd99eb5ac6124a9fd27a7f257f0f8aece5835 Mon Sep 17 00:00:00 2001 From: Samson Date: Fri, 13 Jan 2017 13:57:20 +0800 Subject: [PATCH 1/4] Add input validation for fill_between and fill_betweenx and unit tests --- lib/matplotlib/axes/_axes.py | 13 ++++++ lib/matplotlib/tests/test_axes.py | 72 +++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py index ae08af92fcdf..c4e94f5cec63 100644 --- a/lib/matplotlib/axes/_axes.py +++ b/lib/matplotlib/axes/_axes.py @@ -4757,6 +4757,13 @@ def fill_between(self, x, y1, y2=0, where=None, interpolate=False, for filling between two sets of x-values """ + + for input in [('x', x), ('y1', y1), ('y2', y2)]: + if not (cbook.is_scalar(input[1]) or + cbook.is_scalar(cbook.safe_first_element(input[1]))): + raise ValueError('Input passed into argument "' + input[0] + + '" is not 1-dimensional.') + if not rcParams['_internal.classic_mode']: color_aliases = mcoll._color_aliases kwargs = cbook.normalize_kwargs(kwargs, color_aliases) @@ -4918,6 +4925,12 @@ def fill_betweenx(self, y, x1, x2=0, where=None, for filling between two sets of y-values """ + + for input in [('y', y), ('x1', x1), ('x2', x2), ('where', where)]: + if not cbook.is_scalar(cbook.safe_first_element(input[1])): + raise ValueError('Input passed into argument "' + input[0] + + '" is not 1-dimensional.') + if not rcParams['_internal.classic_mode']: color_aliases = mcoll._color_aliases kwargs = cbook.normalize_kwargs(kwargs, color_aliases) diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py index dde3c5a806ef..d6ae01ce3d92 100644 --- a/lib/matplotlib/tests/test_axes.py +++ b/lib/matplotlib/tests/test_axes.py @@ -725,6 +725,42 @@ def test_polycollection_joinstyle(): ax.set_ybound(0, 3) +@raises(ValueError) +def test_fill_between_2d_x_input(): + x = np.zeros((2, 2)) + y1 = 3 + y2 = 3 + + fig = plt.figure() + ax = fig.add_subplot(211) + ax.plot(x, y1, x, y2, color='black') + ax.fill_between(x, y1, y2) + + +@raises(ValueError) +def test_fill_between_2d_y1_input(): + x = np.arange(0.0, 2, 0.02) + y1 = np.zeros((2, 2)) + y2 = 3 + + fig = plt.figure() + ax = fig.add_subplot(211) + ax.plot(x, y1, x, y2, color='black') + ax.fill_between(x, y1, y2) + + +@raises(ValueError) +def test_fill_between_2d_y2_input(): + x = np.arange(0.0, 2, 0.02) + y1 = 3 + y2 = np.zeros((2, 2)) + + fig = plt.figure() + ax = fig.add_subplot(211) + ax.plot(x, y1, x, y2, color='black') + ax.fill_between(x, y1, y2) + + @image_comparison(baseline_images=['fill_between_interpolate'], remove_text=True) def test_fill_between_interpolate(): @@ -4801,6 +4837,42 @@ def test_tick_param_label_rotation(): assert text.get_rotation() == 90 +@raises(ValueError) +def test_fill_betweenx_2d_y_input(): + y = np.zeros((2, 2)) + x1 = 3 + x2 = 3 + + fig = plt.figure() + ax = fig.add_subplot(211) + ax.plot(y, x1, y, x2, color='black') + ax.fill_betweenx(y, x1, x2) + + +@raises(ValueError) +def test_fill_betweenx_2d_x1_input(): + y = np.arange(0.0, 2, 0.02) + x1 = np.zeros((2, 2)) + x2 = 3 + + fig = plt.figure() + ax = fig.add_subplot(211) + ax.plot(y, x1, y, x2, color='black') + ax.fill_betweenx(y, x1, x2) + + +@raises(ValueError) +def test_fill_betweenx_2d_x2_input(): + y = np.arange(0.0, 2, 0.02) + x1 = 3 + x2 = np.zeros((2, 2)) + + fig = plt.figure() + ax = fig.add_subplot(211) + ax.plot(y, x1, y, x2, color='black') + ax.fill_betweenx(y, x1, x2) + + @cleanup(style='default') def test_fillbetween_cycle(): fig, ax = plt.subplots() From c353bce8981eff4db99421c7e3b2e2307b700207 Mon Sep 17 00:00:00 2001 From: Samson Date: Sat, 14 Jan 2017 16:15:45 +0800 Subject: [PATCH 2/4] Move input validation to below conversion to array --- lib/matplotlib/axes/_axes.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py index c4e94f5cec63..924ecccb078f 100644 --- a/lib/matplotlib/axes/_axes.py +++ b/lib/matplotlib/axes/_axes.py @@ -4758,12 +4758,6 @@ def fill_between(self, x, y1, y2=0, where=None, interpolate=False, """ - for input in [('x', x), ('y1', y1), ('y2', y2)]: - if not (cbook.is_scalar(input[1]) or - cbook.is_scalar(cbook.safe_first_element(input[1]))): - raise ValueError('Input passed into argument "' + input[0] + - '" is not 1-dimensional.') - if not rcParams['_internal.classic_mode']: color_aliases = mcoll._color_aliases kwargs = cbook.normalize_kwargs(kwargs, color_aliases) @@ -4781,6 +4775,11 @@ def fill_between(self, x, y1, y2=0, where=None, interpolate=False, y1 = ma.masked_invalid(self.convert_yunits(y1)) y2 = ma.masked_invalid(self.convert_yunits(y2)) + for name, array in [('x', x), ('y1', y1), ('y2', y2)]: + if array.ndim > 1: + raise ValueError('Input passed into argument "' + name + + '" is not 1-dimensional.') + if y1.ndim == 0: y1 = np.ones_like(x) * y1 if y2.ndim == 0: @@ -4926,11 +4925,6 @@ def fill_betweenx(self, y, x1, x2=0, where=None, """ - for input in [('y', y), ('x1', x1), ('x2', x2), ('where', where)]: - if not cbook.is_scalar(cbook.safe_first_element(input[1])): - raise ValueError('Input passed into argument "' + input[0] + - '" is not 1-dimensional.') - if not rcParams['_internal.classic_mode']: color_aliases = mcoll._color_aliases kwargs = cbook.normalize_kwargs(kwargs, color_aliases) @@ -4947,6 +4941,11 @@ def fill_betweenx(self, y, x1, x2=0, where=None, x1 = ma.masked_invalid(self.convert_xunits(x1)) x2 = ma.masked_invalid(self.convert_xunits(x2)) + for array in [('x', x), ('y1', y1), ('y2', y2)]: + if array[1].ndim > 1: + raise ValueError('Input passed into argument "' + array[0] + + '" is not 1-dimensional.') + if x1.ndim == 0: x1 = np.ones_like(y) * x1 if x2.ndim == 0: From 72f6359c0596faba56dd7809b2ae6573c4d705b0 Mon Sep 17 00:00:00 2001 From: Samson Date: Sat, 14 Jan 2017 16:32:05 +0800 Subject: [PATCH 3/4] Change tests to use pytest.raises() instead of @raises --- lib/matplotlib/tests/test_axes.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py index d6ae01ce3d92..8519e5d950ff 100644 --- a/lib/matplotlib/tests/test_axes.py +++ b/lib/matplotlib/tests/test_axes.py @@ -725,7 +725,6 @@ def test_polycollection_joinstyle(): ax.set_ybound(0, 3) -@raises(ValueError) def test_fill_between_2d_x_input(): x = np.zeros((2, 2)) y1 = 3 @@ -734,10 +733,10 @@ def test_fill_between_2d_x_input(): fig = plt.figure() ax = fig.add_subplot(211) ax.plot(x, y1, x, y2, color='black') - ax.fill_between(x, y1, y2) + with pytest.raises(ValueError): + ax.fill_between(x, y1, y2) -@raises(ValueError) def test_fill_between_2d_y1_input(): x = np.arange(0.0, 2, 0.02) y1 = np.zeros((2, 2)) @@ -746,10 +745,10 @@ def test_fill_between_2d_y1_input(): fig = plt.figure() ax = fig.add_subplot(211) ax.plot(x, y1, x, y2, color='black') - ax.fill_between(x, y1, y2) + with pytest.raises(ValueError): + ax.fill_between(x, y1, y2) -@raises(ValueError) def test_fill_between_2d_y2_input(): x = np.arange(0.0, 2, 0.02) y1 = 3 @@ -758,7 +757,8 @@ def test_fill_between_2d_y2_input(): fig = plt.figure() ax = fig.add_subplot(211) ax.plot(x, y1, x, y2, color='black') - ax.fill_between(x, y1, y2) + with pytest.raises(ValueError): + ax.fill_between(x, y1, y2) @image_comparison(baseline_images=['fill_between_interpolate'], @@ -4837,7 +4837,6 @@ def test_tick_param_label_rotation(): assert text.get_rotation() == 90 -@raises(ValueError) def test_fill_betweenx_2d_y_input(): y = np.zeros((2, 2)) x1 = 3 @@ -4846,10 +4845,10 @@ def test_fill_betweenx_2d_y_input(): fig = plt.figure() ax = fig.add_subplot(211) ax.plot(y, x1, y, x2, color='black') - ax.fill_betweenx(y, x1, x2) + with pytest.raises(ValueError): + ax.fill_betweenx(y, x1, x2) -@raises(ValueError) def test_fill_betweenx_2d_x1_input(): y = np.arange(0.0, 2, 0.02) x1 = np.zeros((2, 2)) @@ -4858,10 +4857,10 @@ def test_fill_betweenx_2d_x1_input(): fig = plt.figure() ax = fig.add_subplot(211) ax.plot(y, x1, y, x2, color='black') - ax.fill_betweenx(y, x1, x2) + with pytest.raises(ValueError): + ax.fill_betweenx(y, x1, x2) -@raises(ValueError) def test_fill_betweenx_2d_x2_input(): y = np.arange(0.0, 2, 0.02) x1 = 3 @@ -4870,7 +4869,8 @@ def test_fill_betweenx_2d_x2_input(): fig = plt.figure() ax = fig.add_subplot(211) ax.plot(y, x1, y, x2, color='black') - ax.fill_betweenx(y, x1, x2) + with pytest.raises(ValueError): + ax.fill_betweenx(y, x1, x2) @cleanup(style='default') From e702cd83592bce8b9d0e6a2217466ad3e252df82 Mon Sep 17 00:00:00 2001 From: Samson Date: Sat, 14 Jan 2017 21:38:18 +0800 Subject: [PATCH 4/4] Update exception string to use %r for formatting --- lib/matplotlib/axes/_axes.py | 12 ++++++------ lib/matplotlib/tests/test_axes.py | 18 ++++++++++++------ 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py index 924ecccb078f..f2ddb3852df3 100644 --- a/lib/matplotlib/axes/_axes.py +++ b/lib/matplotlib/axes/_axes.py @@ -4777,8 +4777,8 @@ def fill_between(self, x, y1, y2=0, where=None, interpolate=False, for name, array in [('x', x), ('y1', y1), ('y2', y2)]: if array.ndim > 1: - raise ValueError('Input passed into argument "' + name + - '" is not 1-dimensional.') + raise ValueError('Input passed into argument "%r"' % name + + 'is not 1-dimensional.') if y1.ndim == 0: y1 = np.ones_like(x) * y1 @@ -4941,10 +4941,10 @@ def fill_betweenx(self, y, x1, x2=0, where=None, x1 = ma.masked_invalid(self.convert_xunits(x1)) x2 = ma.masked_invalid(self.convert_xunits(x2)) - for array in [('x', x), ('y1', y1), ('y2', y2)]: - if array[1].ndim > 1: - raise ValueError('Input passed into argument "' + array[0] + - '" is not 1-dimensional.') + for name, array in [('y', y), ('x1', x1), ('x2', x2)]: + if array.ndim > 1: + raise ValueError('Input passed into argument "%r"' % name + + 'is not 1-dimensional.') if x1.ndim == 0: x1 = np.ones_like(y) * x1 diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py index 8519e5d950ff..6f506c59913d 100644 --- a/lib/matplotlib/tests/test_axes.py +++ b/lib/matplotlib/tests/test_axes.py @@ -725,6 +725,7 @@ def test_polycollection_joinstyle(): ax.set_ybound(0, 3) +@cleanup def test_fill_between_2d_x_input(): x = np.zeros((2, 2)) y1 = 3 @@ -732,11 +733,12 @@ def test_fill_between_2d_x_input(): fig = plt.figure() ax = fig.add_subplot(211) - ax.plot(x, y1, x, y2, color='black') with pytest.raises(ValueError): + ax.plot(x, y1, x, y2, color='black') ax.fill_between(x, y1, y2) +@cleanup def test_fill_between_2d_y1_input(): x = np.arange(0.0, 2, 0.02) y1 = np.zeros((2, 2)) @@ -744,11 +746,12 @@ def test_fill_between_2d_y1_input(): fig = plt.figure() ax = fig.add_subplot(211) - ax.plot(x, y1, x, y2, color='black') with pytest.raises(ValueError): + ax.plot(x, y1, x, y2, color='black') ax.fill_between(x, y1, y2) +@cleanup def test_fill_between_2d_y2_input(): x = np.arange(0.0, 2, 0.02) y1 = 3 @@ -756,8 +759,8 @@ def test_fill_between_2d_y2_input(): fig = plt.figure() ax = fig.add_subplot(211) - ax.plot(x, y1, x, y2, color='black') with pytest.raises(ValueError): + ax.plot(x, y1, x, y2, color='black') ax.fill_between(x, y1, y2) @@ -4837,6 +4840,7 @@ def test_tick_param_label_rotation(): assert text.get_rotation() == 90 +@cleanup def test_fill_betweenx_2d_y_input(): y = np.zeros((2, 2)) x1 = 3 @@ -4844,11 +4848,12 @@ def test_fill_betweenx_2d_y_input(): fig = plt.figure() ax = fig.add_subplot(211) - ax.plot(y, x1, y, x2, color='black') with pytest.raises(ValueError): + ax.plot(y, x1, y, x2, color='black') ax.fill_betweenx(y, x1, x2) +@cleanup def test_fill_betweenx_2d_x1_input(): y = np.arange(0.0, 2, 0.02) x1 = np.zeros((2, 2)) @@ -4856,11 +4861,12 @@ def test_fill_betweenx_2d_x1_input(): fig = plt.figure() ax = fig.add_subplot(211) - ax.plot(y, x1, y, x2, color='black') with pytest.raises(ValueError): + ax.plot(y, x1, y, x2, color='black') ax.fill_betweenx(y, x1, x2) +@cleanup def test_fill_betweenx_2d_x2_input(): y = np.arange(0.0, 2, 0.02) x1 = 3 @@ -4868,8 +4874,8 @@ def test_fill_betweenx_2d_x2_input(): fig = plt.figure() ax = fig.add_subplot(211) - ax.plot(y, x1, y, x2, color='black') with pytest.raises(ValueError): + ax.plot(y, x1, y, x2, color='black') ax.fill_betweenx(y, x1, x2)