From bfe5e3fb84f69045e99ca4337ba1270bf0f8f5b5 Mon Sep 17 00:00:00 2001 From: Benjamin Root Date: Wed, 6 Apr 2016 15:45:31 -0400 Subject: [PATCH 1/2] Improvements to property cycler. Closes #5875 * Allow key normalization to Cycler objects * Validation is performed at more entry points * Requires Cycler v0.9+ --- lib/matplotlib/rcsetup.py | 21 ++++++++++++--------- lib/matplotlib/tests/test_cycles.py | 15 ++++++++++----- lib/matplotlib/tests/test_rcparams.py | 5 +++++ setupext.py | 2 +- 4 files changed, 28 insertions(+), 15 deletions(-) diff --git a/lib/matplotlib/rcsetup.py b/lib/matplotlib/rcsetup.py index 9e46fefcbd3e..fda3a4ddb3fd 100644 --- a/lib/matplotlib/rcsetup.py +++ b/lib/matplotlib/rcsetup.py @@ -726,15 +726,7 @@ def cycler(*args, **kwargs): if not isinstance(args[0], Cycler): raise TypeError("If only one positional argument given, it must " " be a Cycler instance.") - - c = args[0] - unknowns = c.keys - (set(_prop_validators.keys()) | - set(_prop_aliases.keys())) - if unknowns: - # This is about as much validation I can do - raise TypeError("Unknown artist properties: %s" % unknowns) - else: - return Cycler(c) + return validate_cycler(args[0]) elif len(args) == 2: pairs = [(args[0], args[1])] elif len(args) > 2: @@ -789,6 +781,17 @@ def validate_cycler(s): else: raise ValueError("object was not a string or Cycler instance: %s" % s) + unknowns = cycler_inst.keys - (set(_prop_validators.keys()) | + set(_prop_aliases.keys())) + if unknowns: + raise ValueError("Unknown artist properties: %s" % unknowns) + + # Not a full validation, but it'll at least normalize property names + # A fuller validation would require v0.10 of cycler. + for prop in cycler_inst.keys: + norm_prop = _prop_aliases.get(prop, prop) + cycler_inst.change_key(prop, norm_prop) + return cycler_inst diff --git a/lib/matplotlib/tests/test_cycles.py b/lib/matplotlib/tests/test_cycles.py index 5cec742f29ba..e4574f1f1e30 100644 --- a/lib/matplotlib/tests/test_cycles.py +++ b/lib/matplotlib/tests/test_cycles.py @@ -32,7 +32,7 @@ def test_colorcycle_basic(): def test_marker_cycle(): fig = plt.figure() ax = fig.add_subplot(111) - ax.set_prop_cycle(cycler('color', ['r', 'g', 'y']) + + ax.set_prop_cycle(cycler('c', ['r', 'g', 'y']) + cycler('marker', ['.', '*', 'x'])) xs = np.arange(10) ys = 0.25 * xs + 2 @@ -48,7 +48,7 @@ def test_marker_cycle(): fig = plt.figure() ax = fig.add_subplot(111) # Test keyword arguments, numpy arrays, and generic iterators - ax.set_prop_cycle(color=np.array(['r', 'g', 'y']), + ax.set_prop_cycle(c=np.array(['r', 'g', 'y']), marker=iter(['.', '*', 'x'])) xs = np.arange(10) ys = 0.25 * xs + 2 @@ -67,7 +67,7 @@ def test_marker_cycle(): def test_linestylecycle_basic(): fig = plt.figure() ax = fig.add_subplot(111) - ax.set_prop_cycle(cycler('linestyle', ['-', '--', ':'])) + ax.set_prop_cycle(cycler('ls', ['-', '--', ':'])) xs = np.arange(10) ys = 0.25 * xs + 2 ax.plot(xs, ys, label='solid', lw=4) @@ -85,7 +85,7 @@ def test_linestylecycle_basic(): def test_fillcycle_basic(): fig = plt.figure() ax = fig.add_subplot(111) - ax.set_prop_cycle(cycler('color', ['r', 'g', 'y']) + + ax.set_prop_cycle(cycler('c', ['r', 'g', 'y']) + cycler('hatch', ['xx', 'O', '|-']) + cycler('linestyle', ['-', '--', ':'])) xs = np.arange(10) @@ -131,7 +131,7 @@ def test_valid_input_forms(): ax.set_prop_cycle(None) ax.set_prop_cycle(cycler('linewidth', [1, 2])) ax.set_prop_cycle('color', 'rgywkbcm') - ax.set_prop_cycle('linewidth', (1, 2)) + ax.set_prop_cycle('lw', (1, 2)) ax.set_prop_cycle('linewidth', [1, 2]) ax.set_prop_cycle('linewidth', iter([1, 2])) ax.set_prop_cycle('linewidth', np.array([1, 2])) @@ -181,6 +181,11 @@ def test_invalid_input_forms(): 'linewidth', {'1': 1, '2': 2}) assert_raises((TypeError, ValueError), ax.set_prop_cycle, linewidth=1, color='r') + assert_raises((TypeError, ValueError), ax.set_prop_cycle, 'foobar', [1, 2]) + assert_raises((TypeError, ValueError), ax.set_prop_cycle, + foobar=[1, 2]) + assert_raises((TypeError, ValueError), ax.set_prop_cycle, + cycler(foobar=[1, 2])) if __name__ == '__main__': diff --git a/lib/matplotlib/tests/test_rcparams.py b/lib/matplotlib/tests/test_rcparams.py index bc39a50ed22f..b7a965062a31 100644 --- a/lib/matplotlib/tests/test_rcparams.py +++ b/lib/matplotlib/tests/test_rcparams.py @@ -322,6 +322,10 @@ def test_validators(): ("cycler('c', 'rgb') * cycler('linestyle', ['-', '--'])", (cycler('color', 'rgb') * cycler('linestyle', ['-', '--']))), + (cycler('ls', ['-', '--']), + cycler('linestyle', ['-', '--'])), + (cycler(mew=[2, 5]), + cycler('markeredgewidth', [2, 5])), ), # This is *so* incredibly important: validate_cycler() eval's # an arbitrary string! I think I have it locked down enough, @@ -343,6 +347,7 @@ def test_validators(): ('cycler("waka", [1, 2, 3])', ValueError), # not a property ('cycler(c=[1, 2, 3])', ValueError), # invalid values ("cycler(lw=['a', 'b', 'c'])", ValueError), # invalid values + (cycler('waka', [1, 3, 5]), ValueError), # not a property ) }, {'validator': validate_hatch, diff --git a/setupext.py b/setupext.py index 4e5e2e93c335..d7d21549fefb 100755 --- a/setupext.py +++ b/setupext.py @@ -1384,7 +1384,7 @@ def check(self): return "using cycler version %s" % cycler.__version__ def get_install_requires(self): - return ['cycler'] + return ['cycler>=0.9'] class Dateutil(SetupPackage): From 2128c96a0cb40c9abff25de82cf8b99b2db078e2 Mon Sep 17 00:00:00 2001 From: Benjamin Root Date: Fri, 8 Apr 2016 15:39:05 -0400 Subject: [PATCH 2/2] Make the validation mutation-safe, and improve error messages --- lib/matplotlib/rcsetup.py | 20 ++++++++++++++++++-- lib/matplotlib/tests/test_cycles.py | 2 ++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/rcsetup.py b/lib/matplotlib/rcsetup.py index fda3a4ddb3fd..9e357223ff31 100644 --- a/lib/matplotlib/rcsetup.py +++ b/lib/matplotlib/rcsetup.py @@ -781,13 +781,29 @@ def validate_cycler(s): else: raise ValueError("object was not a string or Cycler instance: %s" % s) - unknowns = cycler_inst.keys - (set(_prop_validators.keys()) | - set(_prop_aliases.keys())) + unknowns = cycler_inst.keys - (set(_prop_validators) | set(_prop_aliases)) if unknowns: raise ValueError("Unknown artist properties: %s" % unknowns) # Not a full validation, but it'll at least normalize property names # A fuller validation would require v0.10 of cycler. + checker = set() + for prop in cycler_inst.keys: + norm_prop = _prop_aliases.get(prop, prop) + if norm_prop != prop and norm_prop in cycler_inst.keys: + raise ValueError("Cannot specify both '{0}' and alias '{1}'" + " in the same prop_cycle".format(norm_prop, prop)) + if norm_prop in checker: + raise ValueError("Another property was already aliased to '{0}'." + " Collision normalizing '{1}'.".format(norm_prop, + prop)) + checker.update([norm_prop]) + + # This is just an extra-careful check, just in case there is some + # edge-case I haven't thought of. + assert len(checker) == len(cycler_inst.keys) + + # Now, it should be safe to mutate this cycler for prop in cycler_inst.keys: norm_prop = _prop_aliases.get(prop, prop) cycler_inst.change_key(prop, norm_prop) diff --git a/lib/matplotlib/tests/test_cycles.py b/lib/matplotlib/tests/test_cycles.py index e4574f1f1e30..78a2429249dd 100644 --- a/lib/matplotlib/tests/test_cycles.py +++ b/lib/matplotlib/tests/test_cycles.py @@ -186,6 +186,8 @@ def test_invalid_input_forms(): foobar=[1, 2]) assert_raises((TypeError, ValueError), ax.set_prop_cycle, cycler(foobar=[1, 2])) + assert_raises(ValueError, ax.set_prop_cycle, + cycler(color='rgb', c='cmy')) if __name__ == '__main__':