From 63f4437d645fe7792abd8bc7674526a4b88af1f5 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Tue, 1 Dec 2015 11:30:54 -0500 Subject: [PATCH 1/7] MNT: unify code path of set, update, setp Single code path for the property -> setter lookup logic. --- doc/api/api_changes/2015-12-01-TAC.rst | 12 ++++ lib/matplotlib/artist.py | 83 +++++++++++++------------- 2 files changed, 55 insertions(+), 40 deletions(-) create mode 100644 doc/api/api_changes/2015-12-01-TAC.rst diff --git a/doc/api/api_changes/2015-12-01-TAC.rst b/doc/api/api_changes/2015-12-01-TAC.rst new file mode 100644 index 000000000000..b130e149731d --- /dev/null +++ b/doc/api/api_changes/2015-12-01-TAC.rst @@ -0,0 +1,12 @@ +`Artist.update` has return value +```````````````````````````````` + +The methods `matplotlib.artist.Artist.set`, +`matplotlib.Artist.update`, and the function `matplotlib.artist.setp` +now use a common codepath to look up how to update the given artist +properties (either using the setter methods or an attribute/property). + +The behavior of `matplotlib.Artist.update` is slightly changed to now +sort by key name and returns a list of the returned values from the +setter methods to avoid changing the API of +`matplotlib.Artist.set` and `matplotlib.artist.setp`. diff --git a/lib/matplotlib/artist.py b/lib/matplotlib/artist.py index a4614f214299..591db0d7c3f9 100644 --- a/lib/matplotlib/artist.py +++ b/lib/matplotlib/artist.py @@ -845,21 +845,46 @@ def update(self, props): """ store = self.eventson self.eventson = False - changed = False + try: + ret = [self._update_property(k, v) + for k, v in sorted(props.items(), reverse=True)] + finally: + self.eventson = store - for k, v in six.iteritems(props): - if k in ['axes']: - setattr(self, k, v) - else: - func = getattr(self, 'set_' + k, None) - if func is None or not six.callable(func): - raise AttributeError('Unknown property %s' % k) - func(v) - changed = True - self.eventson = store - if changed: + if len(ret): self.pchanged() self.stale = True + return ret + + def _update_property(self, k, v): + """helper function for set, update, setp + + This function takes care of sorting out if this should be done + through a `set_*` method or a property. + + Parameters + ---------- + k : str + The property to update + + v : obj + The value to assign to the property + + Returns + ------- + ret : obj or None + If using a `set_*` method return it's return, else return None. + """ + k = k.lower() + # white list attributes we want to be able to update through + # art.update, art.set, setp + if k in ['axes']: + return setattr(self, k, v) + else: + func = getattr(self, 'set_' + k, None) + if func is None or not six.callable(func): + raise AttributeError('Unknown property %s' % k) + return func(v) def get_label(self): """ @@ -919,23 +944,9 @@ def properties(self): return ArtistInspector(self).properties() def set(self, **kwargs): + """A property batch setter. Pass *kwargs* to set properties. """ - A property batch setter. Pass *kwargs* to set properties. - Will handle property name collisions (e.g., if both - 'color' and 'facecolor' are specified, the property - with higher priority gets set last). - - """ - ret = [] - for k, v in sorted(kwargs.items(), reverse=True): - k = k.lower() - funcName = "set_%s" % k - func = getattr(self, funcName, None) - if func is None: - raise TypeError('There is no %s property "%s"' % - (self.__class__.__name__, k)) - ret.extend([func(v)]) - return ret + return self.update(kwargs) def findobj(self, match=None, include_self=True): """ @@ -1453,18 +1464,10 @@ def setp(obj, *args, **kwargs): funcvals = [] for i in range(0, len(args) - 1, 2): funcvals.append((args[i], args[i + 1])) - funcvals.extend(sorted(kwargs.items(), reverse=True)) - - ret = [] - for o in objs: - for s, val in funcvals: - s = s.lower() - funcName = "set_%s" % s - func = getattr(o, funcName, None) - if func is None: - raise TypeError('There is no %s property "%s"' % - (o.__class__.__name__, s)) - ret.extend([func(val)]) + # do the *args one at a time to ensure order + ret = [o.update({s: val}) for s, val in funcvals for o in objs] + # apply kwargs in bulk + ret.extend([o.update(kwargs) for o in objs]) return [x for x in cbook.flatten(ret)] From 3401a5d7e04c7738a6dfab108a8849721d653162 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Tue, 1 Dec 2015 11:58:41 -0500 Subject: [PATCH 2/7] MNT: move helper function into update Hide private helper method more thoroughly. --- lib/matplotlib/artist.py | 57 ++++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/lib/matplotlib/artist.py b/lib/matplotlib/artist.py index 591db0d7c3f9..8e5fdd4fc0f2 100644 --- a/lib/matplotlib/artist.py +++ b/lib/matplotlib/artist.py @@ -843,10 +843,35 @@ def update(self, props): Update the properties of this :class:`Artist` from the dictionary *prop*. """ + def _update_property(self, k, v): + """sorting out how to update property (setter or setattr) + + Parameters + ---------- + k : str + The name of property to update + v : obj + The value to assign to the property + Returns + ------- + ret : obj or None + If using a `set_*` method return it's return, else None. + """ + k = k.lower() + # white list attributes we want to be able to update through + # art.update, art.set, setp + if k in {'axes'}: + return setattr(self, k, v) + else: + func = getattr(self, 'set_' + k, None) + if func is None or not six.callable(func): + raise AttributeError('Unknown property %s' % k) + return func(v) + store = self.eventson self.eventson = False try: - ret = [self._update_property(k, v) + ret = [_update_property(self, k, v) for k, v in sorted(props.items(), reverse=True)] finally: self.eventson = store @@ -856,36 +881,6 @@ def update(self, props): self.stale = True return ret - def _update_property(self, k, v): - """helper function for set, update, setp - - This function takes care of sorting out if this should be done - through a `set_*` method or a property. - - Parameters - ---------- - k : str - The property to update - - v : obj - The value to assign to the property - - Returns - ------- - ret : obj or None - If using a `set_*` method return it's return, else return None. - """ - k = k.lower() - # white list attributes we want to be able to update through - # art.update, art.set, setp - if k in ['axes']: - return setattr(self, k, v) - else: - func = getattr(self, 'set_' + k, None) - if func is None or not six.callable(func): - raise AttributeError('Unknown property %s' % k) - return func(v) - def get_label(self): """ Get the label used for this artist in the legend. From c458cbc5321abb90f384a9cb3f7f177f5c11a419 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Tue, 1 Dec 2015 12:00:55 -0500 Subject: [PATCH 3/7] DOC: more minor API constraints --- doc/api/api_changes/2015-12-01-TAC.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/api/api_changes/2015-12-01-TAC.rst b/doc/api/api_changes/2015-12-01-TAC.rst index b130e149731d..642c121a522a 100644 --- a/doc/api/api_changes/2015-12-01-TAC.rst +++ b/doc/api/api_changes/2015-12-01-TAC.rst @@ -10,3 +10,9 @@ The behavior of `matplotlib.Artist.update` is slightly changed to now sort by key name and returns a list of the returned values from the setter methods to avoid changing the API of `matplotlib.Artist.set` and `matplotlib.artist.setp`. + +The keys passed into `matplotlib.Artist.update` are now converted to +all lower case before being processed, to match the behavior of +`matplotlib.Artist.set` and `matplotlib.artist.setp`. This should not +break any user code because there are no set methods with capitals in +the names, however going forward this puts a constraint naming properties. From bba3ae28a0f513e0eb18247d6b587629fa10b986 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Tue, 1 Dec 2015 14:39:04 -0500 Subject: [PATCH 4/7] REV: do not sort by key in Artist.update restore old behavior --- doc/api/api_changes/2015-12-01-TAC.rst | 13 +++++++------ lib/matplotlib/artist.py | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/doc/api/api_changes/2015-12-01-TAC.rst b/doc/api/api_changes/2015-12-01-TAC.rst index 642c121a522a..cc8e11f66570 100644 --- a/doc/api/api_changes/2015-12-01-TAC.rst +++ b/doc/api/api_changes/2015-12-01-TAC.rst @@ -6,13 +6,14 @@ The methods `matplotlib.artist.Artist.set`, now use a common codepath to look up how to update the given artist properties (either using the setter methods or an attribute/property). -The behavior of `matplotlib.Artist.update` is slightly changed to now -sort by key name and returns a list of the returned values from the -setter methods to avoid changing the API of -`matplotlib.Artist.set` and `matplotlib.artist.setp`. +The behavior of `matplotlib.Artist.update` is slightly changed to +returna a list of the returned values from the setter methods to avoid +changing the API of `matplotlib.Artist.set` and +`matplotlib.artist.setp`. The keys passed into `matplotlib.Artist.update` are now converted to -all lower case before being processed, to match the behavior of +lower case before being processed to match the behavior of `matplotlib.Artist.set` and `matplotlib.artist.setp`. This should not break any user code because there are no set methods with capitals in -the names, however going forward this puts a constraint naming properties. +the names, however going forward this puts a constraint naming +properties. diff --git a/lib/matplotlib/artist.py b/lib/matplotlib/artist.py index 8e5fdd4fc0f2..27d77833712d 100644 --- a/lib/matplotlib/artist.py +++ b/lib/matplotlib/artist.py @@ -872,7 +872,7 @@ def _update_property(self, k, v): self.eventson = False try: ret = [_update_property(self, k, v) - for k, v in sorted(props.items(), reverse=True)] + for k, v in props.items()] finally: self.eventson = store From 0ea5fff31083924d9992161a30476e6fbd10e928 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Tue, 1 Dec 2015 14:40:34 -0500 Subject: [PATCH 5/7] MNT: use ordereddict to order args in setp Make one call to `update` with the ordered dict rather than one call per element. --- lib/matplotlib/artist.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/matplotlib/artist.py b/lib/matplotlib/artist.py index 27d77833712d..2dfc1724eb1c 100644 --- a/lib/matplotlib/artist.py +++ b/lib/matplotlib/artist.py @@ -2,6 +2,7 @@ unicode_literals) from matplotlib.externals import six +from collections import OrderedDict import re import warnings @@ -1451,17 +1452,16 @@ def setp(obj, *args, **kwargs): if not cbook.iterable(obj): objs = [obj] else: - objs = cbook.flatten(obj) + objs = list(cbook.flatten(obj)) if len(args) % 2: raise ValueError('The set args must be string, value pairs') - funcvals = [] + # put args into ordereddict to maintain order + funcvals = OrderedDict() for i in range(0, len(args) - 1, 2): - funcvals.append((args[i], args[i + 1])) - # do the *args one at a time to ensure order - ret = [o.update({s: val}) for s, val in funcvals for o in objs] - # apply kwargs in bulk + funcvals[args[i]] = args[i + 1] + ret = [o.update(funcvals) for o in objs] ret.extend([o.update(kwargs) for o in objs]) return [x for x in cbook.flatten(ret)] From 790d9093d667f0f100b73954d6ada552479b7863 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Tue, 1 Dec 2015 14:44:14 -0500 Subject: [PATCH 6/7] ENH: add property precedence Add the ability to set the precedence of properties when updating multiple properties on once. This adds a class-level attribute `Artist._prop_order` which is a dictionary keyed on property names with integer values. When using `set` or `setp` the properties will be applied in descending order by value then by name. --- lib/matplotlib/artist.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/matplotlib/artist.py b/lib/matplotlib/artist.py index 2dfc1724eb1c..60fc18929037 100644 --- a/lib/matplotlib/artist.py +++ b/lib/matplotlib/artist.py @@ -83,6 +83,10 @@ class Artist(object): aname = 'Artist' zorder = 0 + # order of precedence when bulk setting/updating properties + # via update. The keys should be property names and the values + # integers + _prop_order = dict(color=-1) def __init__(self): self._stale = True @@ -942,7 +946,11 @@ def properties(self): def set(self, **kwargs): """A property batch setter. Pass *kwargs* to set properties. """ - return self.update(kwargs) + props = OrderedDict( + sorted(kwargs.items(), reverse=True, + key=lambda x: (self._prop_order.get(x[0], 0), x[0]))) + + return self.update(props) def findobj(self, match=None, include_self=True): """ @@ -1461,8 +1469,9 @@ def setp(obj, *args, **kwargs): funcvals = OrderedDict() for i in range(0, len(args) - 1, 2): funcvals[args[i]] = args[i + 1] + ret = [o.update(funcvals) for o in objs] - ret.extend([o.update(kwargs) for o in objs]) + ret.extend([o.set(**kwargs) for o in objs]) return [x for x in cbook.flatten(ret)] From 1905607320e6df162da2070168779105e1a9eaa6 Mon Sep 17 00:00:00 2001 From: Thomas A Caswell Date: Tue, 1 Dec 2015 15:01:01 -0500 Subject: [PATCH 7/7] DOC: fix typos --- doc/api/api_changes/2015-12-01-TAC.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/api_changes/2015-12-01-TAC.rst b/doc/api/api_changes/2015-12-01-TAC.rst index cc8e11f66570..62ad44cec06e 100644 --- a/doc/api/api_changes/2015-12-01-TAC.rst +++ b/doc/api/api_changes/2015-12-01-TAC.rst @@ -7,7 +7,7 @@ now use a common codepath to look up how to update the given artist properties (either using the setter methods or an attribute/property). The behavior of `matplotlib.Artist.update` is slightly changed to -returna a list of the returned values from the setter methods to avoid +return a list of the returned values from the setter methods to avoid changing the API of `matplotlib.Artist.set` and `matplotlib.artist.setp`. @@ -15,5 +15,5 @@ The keys passed into `matplotlib.Artist.update` are now converted to lower case before being processed to match the behavior of `matplotlib.Artist.set` and `matplotlib.artist.setp`. This should not break any user code because there are no set methods with capitals in -the names, however going forward this puts a constraint naming +the names, however going forward this puts a constraint on naming properties.