Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Single axes artist #3835

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 4 commits into from
Dec 13, 2014
Merged

Single axes artist #3835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fa87fa7
TST : tweak test_add_collection
tacaswell Nov 23, 2014
394447f
MNT : fix `__init__` order in AnchoredSizeLocator
tacaswell Nov 24, 2014
69decec
MNT : tweak how internals of Axes.plot work
tacaswell Nov 27, 2014
c137a71
API : property-ify Artist.axes
tacaswell Nov 23, 2014
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
16 changes: 16 additions & 0 deletions 16 doc/api/api_changes/2014-12-12_axes_property.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
Prevent moving artists between Axes, Property-ify Artist.axes, deprecate Artist.{get,set}_axes
``````````````````````````````````````````````````````````````````````````````````````````````

The reason this was done was to prevent adding an Artist that is
already associated with an Axes to be moved/added to a different Axes.
This was never supported as it causes havoc with the transform stack.
The apparent support for this (as it did not raise an exception) was
the source of multiple bug reports and questions on SO.

For almost all use-cases, the assignment of the axes to an artist should be
taken care of by the axes as part of the ``Axes.add_*`` method, hence the
deprecation {get,set}_axes.

Removing the ``set_axes`` method will also remove the 'axes' line from
the ACCEPTS kwarg tables (assuming that the removal date gets here
before that gets overhauled).
44 changes: 39 additions & 5 deletions 44 lib/matplotlib/artist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import inspect
import matplotlib
import matplotlib.cbook as cbook
from matplotlib.cbook import mplDeprecation
from matplotlib import docstring, rcParams
from .transforms import (Bbox, IdentityTransform, TransformedBbox,
TransformedPath, Transform)
Expand Down Expand Up @@ -77,6 +78,7 @@ class Artist(object):
zorder = 0

def __init__(self):
self._axes = None
self.figure = None

self._transform = None
Expand Down Expand Up @@ -175,17 +177,43 @@ def set_axes(self, axes):
Set the :class:`~matplotlib.axes.Axes` instance in which the
artist resides, if any.

This has been deprecated in mpl 1.5, please use the
Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% comfortable with the deprecation. It is a fairly major API chance, no?

Copy link
Member

Choose a reason for hiding this comment

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

I am ok with it. I am wary of the open-ended schedule for removal, though. It would be better to simply schedule its removal a few releases from now, even if we can be a little bit vague on the exact release number at the moment.

Copy link
Member Author

Choose a reason for hiding this comment

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

I view this as an implementation detail which should only be called from the add_* methods on Axes (by c++ background is leaking through, Axes and Artist are friends) and probably never should have been part of the public API to begin with.

I was being as hedgey as possible in the removal date, I am in favor of doing it as soon as possible.

Copy link
Member

Choose a reason for hiding this comment

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

probably never should have been part of the public API to begin with.

property becomes _axes then?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exposing it read-only probably is useful, maybe the setter method should be turned into _set_axes?

Copy link
Member Author

Choose a reason for hiding this comment

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

I looked into removing the setter and using a _set_axes instead and it touches a lot of places where we currently just reach and touch the attribute.

axes property. Will be removed in 1.7 or 2.0.

ACCEPTS: an :class:`~matplotlib.axes.Axes` instance
"""
warnings.warn(_get_axes_msg, mplDeprecation, stacklevel=1)
self.axes = axes

def get_axes(self):
"""
Return the :class:`~matplotlib.axes.Axes` instance the artist
resides in, or *None*
resides in, or *None*.

This has been deprecated in mpl 1.5, please use the
axes property. Will be removed in 1.7 or 2.0.
"""
warnings.warn(_get_axes_msg, mplDeprecation, stacklevel=1)
return self.axes

@property
def axes(self):
"""
The :class:`~matplotlib.axes.Axes` instance the artist
resides in, or *None*.
"""
return self._axes

@axes.setter
def axes(self, new_axes):
if self._axes is not None and new_axes != self._axes:
raise ValueError("Can not reset the axes. You are "
"probably trying to re-use an artist "
"in more than one Axes which is not "
"supported")
self._axes = new_axes
return new_axes

def get_window_extent(self, renderer):
"""
Get the axes bounding box in display space.
Expand Down Expand Up @@ -751,10 +779,13 @@ def update(self, props):
changed = False

for k, v in six.iteritems(props):
func = getattr(self, 'set_' + k, None)
if func is None or not six.callable(func):
raise AttributeError('Unknown property %s' % k)
func(v)
if k in ['axes']:
Copy link
Member

Choose a reason for hiding this comment

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

Why use this "in" test when the list has only one entry?

Copy link
Member Author

Choose a reason for hiding this comment

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

Forward planning for when more of the set_* methods get propertified.

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:
Expand Down Expand Up @@ -1328,3 +1359,6 @@ def kwdoc(a):
return '\n'.join(ArtistInspector(a).pprint_setters(leadingspace=2))

docstring.interpd.update(Artist=kwdoc(Artist))

_get_axes_msg = """This has been deprecated in mpl 1.5, please use the
axes property. A removal date has not been set."""
8 changes: 4 additions & 4 deletions 8 lib/matplotlib/axes/_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,6 @@ def _makeline(self, x, y, kw, kwargs):
# (can't use setdefault because it always evaluates
# its second argument)
seg = mlines.Line2D(x, y,
axes=self.axes,
**kw
)
self.set_lineprops(seg, **kwargs)
Expand Down Expand Up @@ -393,7 +392,8 @@ def __init__(self, fig, rect,
else:
self._position = mtransforms.Bbox.from_bounds(*rect)
self._originalPosition = self._position.frozen()
self.set_axes(self)
# self.set_axes(self)
self.axes = self
self.set_aspect('auto')
self._adjustable = 'box'
self.set_anchor('C')
Expand Down Expand Up @@ -775,7 +775,7 @@ def _set_artist_props(self, a):
if not a.is_transform_set():
a.set_transform(self.transData)

a.set_axes(self)
a.axes = self

def _gen_axes_patch(self):
"""
Expand Down Expand Up @@ -1432,7 +1432,7 @@ def add_artist(self, a):

Returns the artist.
"""
a.set_axes(self)
a.axes = self
self.artists.append(a)
self._set_artist_props(a)
a.set_clip_path(self.patch)
Expand Down
6 changes: 5 additions & 1 deletion 6 lib/matplotlib/figure.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,11 @@ def __init__(self,
Defaults to rc ``figure.autolayout``.
"""
Artist.__init__(self)

# remove the non-figure artist _axes property
# as it makes no sense for a figure to be _in_ an axes
# this is used by the property methods in the artist base class
# which are over-ridden in this class
del self._axes
Copy link
Member

Choose a reason for hiding this comment

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

Does it also make sense then to remove the axes property at the same time? It seems odd to leave it in place while yanking its private variable out from under it.

Copy link
Member Author

Choose a reason for hiding this comment

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

It already re-defines the property on https://github.com/matplotlib/matplotlib/pull/3835/files#diff-c3ce78ddcc28c54e04ade080c149c1b1R397 where it is overloaded to mean 'the list of axes this figure contains'.

Copy link
Member

Choose a reason for hiding this comment

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

Oops! Of course...

self.callbacks = cbook.CallbackRegistry()

if figsize is None:
Expand Down
6 changes: 4 additions & 2 deletions 6 lib/matplotlib/legend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ def __init__(self, parent, handles, labels,

if isinstance(parent, Axes):
self.isaxes = True
self.set_axes(parent)
self.axes = parent
self.set_figure(parent.figure)
elif isinstance(parent, Figure):
self.isaxes = False
Expand Down Expand Up @@ -391,7 +391,9 @@ def _set_artist_props(self, a):
"""
a.set_figure(self.figure)
if self.isaxes:
a.set_axes(self.axes)
# a.set_axes(self.axes)
a.axes = self.axes

a.set_transform(self.get_transform())

def _set_loc(self, loc):
Expand Down
8 changes: 5 additions & 3 deletions 8 lib/matplotlib/lines.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,15 +536,17 @@ def get_window_extent(self, renderer):
bbox = bbox.padded(ms)
return bbox

def set_axes(self, ax):
Artist.set_axes(self, ax)
@Artist.axes.setter
def axes(self, ax):
# call the set method from the base-class property
Artist.axes.fset(self, ax)
# connect unit-related callbacks
if ax.xaxis is not None:
self._xcid = ax.xaxis.callbacks.connect('units',
self.recache_always)
if ax.yaxis is not None:
self._ycid = ax.yaxis.callbacks.connect('units',
self.recache_always)
set_axes.__doc__ = Artist.set_axes.__doc__

def set_data(self, *args):
"""
Expand Down
8 changes: 3 additions & 5 deletions 8 lib/matplotlib/tests/test_collections.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,14 +407,12 @@ def test_null_collection_datalim():
def test_add_collection():
# Test if data limits are unchanged by adding an empty collection.
# Github issue #1490, pull #1497.
ax = plt.axes()
plt.figure()
ax2 = plt.axes()
coll = ax2.scatter([0, 1], [0, 1])
ax = plt.axes()
coll = ax.scatter([0, 1], [0, 1])
ax.add_collection(coll)
bounds = ax.dataLim.bounds
coll = ax2.scatter([], [])
ax.add_collection(coll)
coll = ax.scatter([], [])
assert_equal(ax.dataLim.bounds, bounds)


Expand Down
7 changes: 4 additions & 3 deletions 7 lib/mpl_toolkits/axes_grid1/inset_locator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,21 @@ def __call__(self, ax, renderer):

return bb


from . import axes_size as Size


class AnchoredSizeLocator(AnchoredLocatorBase):
def __init__(self, bbox_to_anchor, x_size, y_size, loc,
borderpad=0.5, bbox_transform=None):
self.axes = None
self.x_size = Size.from_any(x_size)
self.y_size = Size.from_any(y_size)

super(AnchoredSizeLocator, self).__init__(bbox_to_anchor, None, loc,
borderpad=borderpad,
bbox_transform=bbox_transform)

self.x_size = Size.from_any(x_size)
self.y_size = Size.from_any(y_size)

def get_extent(self, renderer):

x, y, w, h = self.get_bbox_to_anchor().bounds
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.