8000 Declare property aliases in a single place by anntzer · Pull Request #9475 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Declare property aliases in a single place #9475

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
Mar 15, 2018
Merged

Declare property aliases in a single place #9475

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
ef7a035
Declare all property aliases together.
anntzer Oct 18, 2017
413144a
Reuse the alias map.
anntzer Oct 18, 2017
7c40fdf
Fix some problematic aliases.
anntzer Oct 18, 2017
a7f698e
Minor style changes.
anntzer Dec 15, 2017
b442049
Drop need for Py2 unicode hack.
anntzer Dec 28, 2017
2c83bdb
Make _define_aliases a class decorator.
anntzer Jan 1, 2018
473590c
Error out in case of alias inheritance.
anntzer Jan 2, 2018
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
Prev Previous commit
Next Next commit
Make _define_aliases a class decorator.
  • Loading branch information
@anntzer
anntzer committed Feb 2, 2018
commit 2c83bdbe6aa149490da67e3f500cc08ac1bbfd26
20 changes: 11 additions & 9 deletions lib/matplotlib/cbook/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2805,14 +2805,13 @@ def _str_lower_equal(obj, s):
return isinstance(obj, six.string_types) and obj.lower() == s


def _define_aliases(local_d, alias_d):
"""Define property aliases.
def _define_aliases(alias_d, cls=None):
"""Class decorator for defining property aliases.

Use in a class definition as ::
Use as ::

cbook._define_aliases(locals(), {
"property": ["alias", ...], ...
})
@cbook._define_aliases({"property": ["alias", ...], ...})
class C: ...

For each property, if the corresponding ``get_property`` is defined in the
class so far, an alias named ``get_alias`` will be defined; the same will
Expand All @@ -2822,6 +2821,8 @@ class so far, an alias named ``get_alias`` will be defined; the same will
The alias map is stored as the ``_alias_map`` attribute on the class and
can be used by `~.normalize_kwargs`.
"""
if cls is None:
return functools.partial(_define_aliases, alias_d)

def make_alias(name): # Enfore a closure over *name*.
Copy link

Choose a reason for hiding this comment

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

typo enforce?

def method(self, *args, **kwargs):
Expand All @@ -2831,14 +2832,15 @@ def method(self, *args, **kwargs):
for prop, aliases in alias_d.items():
exists = False
for prefix in ["get_", "set_"]:
if prefix + prop in local_d:
if prefix + prop in vars(cls):
exists = True
for alias in aliases:
method = make_alias(prefix + prop)
method.__name__ = prefix + alias
method.__doc__ = "alias for `{}`".format(prefix + prop)
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose setting __signature__ through functools.wraps also does the wrong thing for overriden properties...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm mostly too lazy to add a dep on funcsigs for this PR (I actually do add such a dependency for the pyplot PR, so that'd also mean merge conflicts and sadness).

local_d[prefix + alias] = method
setattr(cls, prefix + alias, method)
if not exists:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is more explicit than the any(...) you propose.

raise ValueError("property {} does not exist".format(prop))
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a different word than 'property' here? They are not python @property instances which may lead to confusion.

Copy link
Contributor
@eric-wieser eric-wieser Jan 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

How about "getter or setter methods for {!r} do not exist"?

edit: or "neither getter nor setter methods for {!r} exist"


local_d["_alias_map"] = alias_d
cls._alias_map = alias_d
Copy link
Contributor
@eric-wieser eric-wieser Jan 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I might be tempted to have _define_aliases read this directly, rather than include it in the decorator argument and have it set it anyway - that means the aliases come after the definitions too, and that it becomes more obvious where _alias_map comes from when calling normalize_kwargs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really would prefer the aliases to stay close to the decorator, at least.

Copy link
Member
@tacaswell tacaswell Jan 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I prefer it this way, to keep the decorator and the alias map close together.

Would it make sense to have the decorator also add a _normalize_kw method as

def normalize_kvwargs(self, kw):
    return normalize_kwargs(kw, self._alias_map)

?

Might also be better to check if the class already has an _alias_map and if so recursively update it so that the decorator can be used on sub-classes.

Copy link
Contributor

Choose a reason for hiding this comment

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

if so recursively update it

What you really want here is a collections.ChainMap, but it's not there in python 2.7.

Copy link
Contributor
@eric-wieser eric-wieser Jan 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Or maybe

@staticmethod
def _get_alias_map(self):
    d = {}
    for cls in self.mro():
        for k, aliases in cls.__dict__.get('_alias_map',{}).items()
            d.setdefault(k, []).extend(aliases)

edit: updated

Copy link
Member

Choose a reason for hiding this comment

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

I don't think ChainMap helps here, ex

@_define_aliases({'linewidth': ['lw']})
class A:
   ...

@_define_aliases({'linewidth': ['linewidths', 'lws']})
class B(A):
   ...

You would want B._alias_map['linewidth'] == ['lw', 'linewidths', 'lws']

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. Updated my other comment to reflect that update doesn't work either

return cls
14 changes: 7 additions & 7 deletions lib/matplotlib/collections.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@
CIRCLE_AREA_FACTOR = 1.0 / np.sqrt(np.pi)


@cbook._define_aliases({
"antialiased": ["antialiaseds"],
"edgecolor": ["edgecolors"],
"facecolor": ["facecolors"],
"linestyle": ["linestyles", "dashes"],
"linewidth": ["linewidths", "lw"],
Copy link
Contributor
@eric-wieser eric-wieser Jan 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Why not sets, instead of lists? Duplicates wouldn't make any sense here. (edit: because normalize_kwargs requires it to be so)

})
class Collection(artist.Artist, cm.ScalarMappable):
"""
Base class for Collections. Must be subclassed to be usable.
Expand Down Expand Up @@ -799,13 +806,6 @@ def update_from(self, other):
# self.update_dict = other.update_dict # do we need to copy this? -JJL
self.stale = True

cbook._define_aliases(locals(), {
"antialiased": ["antialiaseds"],
"edgecolor": ["edgecolors"],
"facecolor": ["facecolors"],
"linestyle": ["linestyles", "dashes"],
"linewidth": ["linewidths", "lw"],
})

# these are not available for the object inspector until after the
# class is built so we define an initial set here for the init
Expand Down
26 changes: 12 additions & 14 deletions lib/matplotlib/lines.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,15 +229,25 @@ def _slice_or_none(in_v, slc):
'markevery=%s' % (markevery,))


@cbook._define_aliases({
"antialiased": ["aa"],
"color": ["c"],
"linestyle": ["ls"],
"linewidth": ["lw"],
"markeredgecolor": ["mec"],
"markeredgewidth": ["mew"],
"markerfacecolor": ["mfc"],
"markerfacecoloralt": ["mfcalt"],
"markersize": ["ms"],
})
class Line2D(Artist):
"""
A line - the line can have both a solid linestyle connecting all
the vertices, and a marker at each vertex. Additionally, the
drawing of the solid line is influenced by the drawstyle, e.g., one
can create "stepped" lines in various styles.


"""

lineStyles = _lineStyles = { # hidden names deprecated
'-': '_draw_solid',
'--': '_draw_dashed',
Expand Down Expand Up @@ -1347,18 +1357,6 @@ def is_dashed(self):
'return True if line is dashstyle'
return self._linestyle in ('--', '-.', ':')

cbook._define_aliases(locals(), {
"antialiased": ["aa"],
"color": ["c"],
"linestyle": ["ls"],
"linewidth": ["lw"],
"markeredgecolor": ["mec"],
"markeredgewidth": ["mew"],
"markerfacecolor": ["mfc"],
"markerfacecoloralt": ["mfcalt"],
"markersize": ["ms"],
})


class VertexSelector(object):
"""
Expand Down
16 changes: 8 additions & 8 deletions lib/matplotlib/patches.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@
split_bezier_intersecting_with_closedpath, split_path_inout)
from .path import Path


@cbook._define_aliases({
"antialiased": ["aa"],
"edgecolor": ["ec"],
"facecolor": ["fc"],
"linewidth": ["lw"],
"linestyle": ["ls"],
})
class Patch(artist.Artist):
"""
A patch is a 2D artist with a face color and an edge color.
Expand Down Expand Up @@ -538,14 +546,6 @@ def get_path(self):
def get_window_extent(self, renderer=None):
return self.get_path().get_extents(self.get_transform())

cbook._define_aliases(locals(), {
"antialiased": ["aa"],
"edgecolor": ["ec"],
"facecolor": ["fc"],
"linewidth": ["lw"],
"linestyle": ["ls"],
})


patchdoc = artist.kwdoc(Patch)
for k in ('Rectangle', 'Circle', 'RegularPolygon', 'Polygon', 'Wedge', 'Arrow',
Expand Down
26 changes: 13 additions & 13 deletions lib/matplotlib/text.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,19 @@ def _get_textbox(text, renderer):
return x_box, y_box, w_box, h_box


@cbook._define_aliases({
"family": ["fontfamily"],
"fontproperties": ["font_properties"],
"horizontalalignment": ["ha"],
"multialignment": ["ma"],
"name": ["fontname"],
"size": ["fontsize"],
"stretch": ["fontstretch"],
"style": ["fontstyle"],
"variant": ["fontvariant"],
"verticalalignment": ["va"],
"weight": ["fontweight"],
})
class Text(Artist):
"""
Handle storing and drawing of text in window or data coordinates.
Expand Down Expand Up @@ -1154,19 +1167,6 @@ def set_name(self, fontname): # One-way alias only: the getter differs.
"""alias for set_family"""
return self.set_family(fontname)

cbook._define_aliases(locals(), {
"family": ["fontfamily"],
"fontproperties": ["font_properties"],
"horizontalalignment": ["ha"],
"multialignment": ["ma"],
"name": ["fontname"],
"size": ["fontsize"],
"stretch": ["fontstretch"],
"style": ["fontstyle"],
"variant": ["fontvariant"],
"verticalalignment": ["va"],
"weight": ["fontweight"],
})

docstring.interpd.update(Text=artist.kwdoc(Text))
docstring.dedent_interpd(Text.__init__)
Expand Down
0