8000 MNT: unify code path of set, update, setp by tacaswell · Pull Request #5599 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

MNT: unify code path of set, update, setp #5599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 14, 2015
Merged

MNT: unify code path of set, update, setp #5599

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
63f4437
MNT: unify code path of set, update, setp
tacaswell Dec 1, 2015
3401a5d
MNT: move helper function into update
tacaswell Dec 1, 2015
c458cbc
DOC: more minor API constraints
tacaswell Dec 1, 2015
bba3ae2
REV: do not sort by key in Artist.update
tacaswell Dec 1, 2015
0ea5fff
MNT: use ordereddict to order args in setp
tacaswell Dec 1, 2015
790d909
ENH: add property precedence
tacaswell Dec 1, 2015
1905607
DOC: fix typos
tacaswell Dec 1, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
MNT: unify code path of set, update, setp 
Single code path for the property -> setter lookup logic.
  • Loading branch information
@tacaswell
tacaswell committed Dec 1, 2015
commit 63f4437d645fe7792abd8bc7674526a4b88af1f5
12 changes: 12 additions & 0 deletions doc/api/api_changes/2015-12-01-TAC.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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`.
83 changes: 43 additions & 40 deletions lib/matplotlib/artist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sorting by key name is just to prevent any non-deterministic behavior?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and makes color/facecolor/edgecolor go in the right order.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partly. It is also to have a priority structure. If someone defines both
"color" and "facecolor" at the same time, the current behavior is for color
to take precedence over facecolor, IIRC.

On Tue, Dec 1, 2015 at 11:51 AM, Michael Droettboom <
notifications@github.com> wrote:

In lib/matplotlib/artist.py
#5599 (comment):

@@ -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)]

The sorting by key name is just to prevent any non-deterministic behavior?

—
Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/5599/files#r46304017.

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):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this could be an embedded function in update(), since it isn't used from anywhere else?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, when I started I didn't think I would be able to use update everywhere else.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on second thought, wouldn't that result in re-defining the function everytime through call to update?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No -- Python will compile the function only once -- it only redefines the local variables each time.

"""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):
"""
Expand Down Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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)]


Expand Down
0