Skip to content

ENH: Switch to a private, simpler AxesStack. #7377

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.

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 1 commit into from
Aug 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
108 changes: 66 additions & 42 deletions lib/matplotlib/figure.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class AxesStack(Stack):

"""
def __init__(self):
cbook.warn_deprecated("2.1")
Stack.__init__(self)
self._ind = 0

Expand Down Expand Up @@ -158,6 +159,62 @@ def __contains__(self, a):
return a in self.as_list()


class _AxesStack(object):
"""Lightweight stack that tracks Axes in a Figure.
"""

def __init__(self):
# We maintain a list of (creation_index, key, axes) tuples.
# We do not use an OrderedDict because 1. the keys may not be hashable
# and 2. we need to directly find a pair corresponding to an axes (i.e.
# we'd really need a two-way dict).
self._items = []
self._created = 0

def as_list(self):
"""Copy of the list of axes, in the order of insertion.
"""
return [ax for _, _, ax in sorted(self._items)]
Copy link
Member

Choose a reason for hiding this comment

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

I know it's originally sorted this way, but I wonder if that's even necessary...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The "keep creation order" behavior is mandated by the tests right now, and I don't see a reason to get rid of it.


def get(self, key):
"""Find the axes corresponding to a key; defaults to `None`.
"""
return next((ax for _, k, ax in self._items if k == key), None)
Copy link
Member

Choose a reason for hiding this comment

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

@anntzer I consistently learn new python details from your code.


def current_key_axes(self):
"""Return the topmost `(key, axes)` pair, or `(None, None)` if empty.
"""
_, key, ax = (self._items or [(None, None, None)])[-1]
return key, ax

def add(self, key, ax):
"""Append a `(key, axes)` pair, unless the axes are already present.
"""
# Skipping existing Axes is needed to support calling `add_axes` with
# an already existing Axes.
if not any(a == ax for _, _, a in self._items):
Copy link
Member

Choose a reason for hiding this comment

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

This probably needs to handle the duplicate key case?

Copy link
Contributor Author

@anntzer anntzer Nov 11, 2016

Choose a reason for hiding this comment

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

Each call to add is actually preceded by a call to get, so technically we can rely on the caller code to never try to add in a duplicate key. Also I worded the docstring carefully: the pair is not appended if the axes is already present but there's no check on whether the key is :-)

In practice I could make the behavior (no checking for duplicate key) clearer in the docstring, or add a check and error out if the key is already there; either is fine with me. Please let me know your preference.

self._items.append((self._created, key, ax))
self._created += 1

def bubble(self, ax):
"""Move an axes and its corresponding key to the top.
"""
idx, = (idx for idx, (_, _, a) in enumerate(self._items) if a == ax)
self._items.append(self._items[idx])
del self._items[idx]

def remove(self, ax):
"""Remove an axes and its corresponding key.
"""
idx, = (idx for idx, (_, _, a) in enumerate(self._items) if a == ax)
del self._items[idx]

def clear(self):
"""Clear the stack.
"""
del self._items[:]


class SubplotParams(object):
"""
A class to hold the parameters for a subplot
Expand Down Expand Up @@ -358,7 +415,7 @@ def __init__(self,
self.subplotpars = subplotpars
self.set_tight_layout(tight_layout)

self._axstack = AxesStack() # track all figure axes and current axes
self._axstack = _AxesStack() # track all figure axes and current axes
self.clf()
self._cachedRenderer = None

Expand Down Expand Up @@ -410,10 +467,8 @@ def show(self, warn=True):
"matplotlib is currently using a non-GUI backend, "
"so cannot show the figure")

def _get_axes(self):
return self._axstack.as_list()

axes = property(fget=_get_axes, doc="Read-only: list of axes in Figure")
axes = property(lambda self: self._axstack.as_list(),
Copy link
Member

Choose a reason for hiding this comment

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

Can't this be

    @property
    def axes(self):
        """Read-only: list of axes in Figure"""
        return self._axstack.as_list()

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 don't think one is more readable that the other?

doc="Read-only: list of axes in Figure")

def _get_dpi(self):
return self._dpi
Expand Down Expand Up @@ -835,36 +890,6 @@ def delaxes(self, a):
func(self)
self.stale = True

def _make_key(self, *args, **kwargs):
'make a hashable key out of args and kwargs'

def fixitems(items):
#items may have arrays and lists in them, so convert them
# to tuples for the key
ret = []
for k, v in items:
# some objects can define __getitem__ without being
# iterable and in those cases the conversion to tuples
# will fail. So instead of using the iterable(v) function
# we simply try and convert to a tuple, and proceed if not.
try:
v = tuple(v)
except Exception:
pass
ret.append((k, v))
return tuple(ret)

def fixlist(args):
ret = []
for a in args:
if iterable(a):
a = tuple(a)
ret.append(a)
return tuple(ret)

key = fixlist(args), fixitems(six.iteritems(kwargs))
return key

def add_axes(self, *args, **kwargs):
"""
Add an axes at position *rect* [*left*, *bottom*, *width*,
Expand Down Expand Up @@ -929,9 +954,9 @@ def add_axes(self, *args, **kwargs):

# shortcut the projection "key" modifications later on, if an axes
# with the exact args/kwargs exists, return it immediately.
key = self._make_key(*args, **kwargs)
key = (args, kwargs)
ax = self._axstack.get(key)
if ax is not None:
if ax:
self.sca(ax)
return ax

Expand All @@ -951,7 +976,7 @@ def add_axes(self, *args, **kwargs):
# check that an axes of this type doesn't already exist, if it
# does, set it as active and return it
ax = self._axstack.get(key)
if ax is not None and isinstance(ax, projection_class):
if isinstance(ax, projection_class):
self.sca(ax)
return ax

Expand Down Expand Up @@ -1037,15 +1062,14 @@ def add_subplot(self, *args, **kwargs):
raise ValueError(msg)
# make a key for the subplot (which includes the axes object id
# in the hash)
key = self._make_key(*args, **kwargs)
key = (args, kwargs)
else:
projection_class, kwargs, key = process_projection_requirements(
self, *args, **kwargs)

# try to find the axes with this key in the stack
ax = self._axstack.get(key)

if ax is not None:
if ax:
if isinstance(ax, projection_class):
# the axes already existed, so set it as active & return
self.sca(ax)
Expand Down Expand Up @@ -1614,7 +1638,7 @@ def _gci(self):
do not use elsewhere.
"""
# Look first for an image in the current Axes:
cax = self._axstack.current_key_axes()[1]
ckey, cax = self._axstack.current_key_axes()
if cax is None:
return None
im = cax._gci()
Expand Down
6 changes: 1 addition & 5 deletions lib/matplotlib/projections/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,7 @@ def process_projection_requirements(figure, *args, **kwargs):
raise TypeError('projection must be a string, None or implement a '
'_as_mpl_axes method. Got %r' % projection)

# Make the key without projection kwargs, this is used as a unique
# lookup for axes instances
key = figure._make_key(*args, **kwargs)

return projection_class, kwargs, key
return projection_class, kwargs, (args, kwargs)


def get_projection_names():
Expand Down