Skip to content

Prevent reuse of certain Locators and Formatters over multiple Axises. #13439

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

Closed
Closed
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
17 changes: 17 additions & 0 deletions doc/api/next_api_changes/2018-02-15-AL.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
Axis-dependent Locators and Formatters explicitly error out when used over multiple Axis
````````````````````````````````````````````````````````````````````````````````````````

Certain Locators and Formatters (e.g. the default `AutoLocator` and
`ScalarFormatter`) can only be used meaningfully on one Axis object at a time
(i.e., attempting to use a single `AutoLocator` instance on the x and the y
axis of an Axes, or the x axis of two different Axes, would result in
nonsensical results).

Such "double-use" is now detected and raises a RuntimeError *at canvas draw
time*. The exception is not raised when the second Axis is registered in order
to avoid incorrectly raising exceptions for the Locators and Formatters that
*can* be used on multiple Axis objects simultaneously (e.g. `NullLocator` and
`FuncFormatter`).

In case a Locator or a Formatter really needs to be reassigned from one axis to
another, first set its axis to None to bypass this protection.
22 changes: 15 additions & 7 deletions lib/matplotlib/figure.py
Original file line number Diff line number Diff line change
Expand Up @@ -1589,11 +1589,19 @@ def subplots(self, nrows=1, ncols=1, sharex=False, sharey=False,
return axarr

def _remove_ax(self, ax):
def _reset_loc_form(axis):
axis.set_major_formatter(axis.get_major_formatter())
axis.set_major_locator(axis.get_major_locator())
axis.set_minor_formatter(axis.get_minor_formatter())
axis.set_minor_locator(axis.get_minor_locator())
def _reset_tickers(axis):
major_formatter = axis.get_major_formatter()
major_formatter.set_axis(None) # Bypass prevention of axis reset.
axis.set_major_formatter(major_formatter)
major_locator = axis.get_major_locator()
major_locator.set_axis(None)
axis.set_major_locator(major_locator)
minor_formatter = axis.get_minor_formatter()
minor_formatter.set_axis(None)
axis.set_minor_formatter(minor_formatter)
minor_locator = axis.get_minor_locator()
minor_locator.set_axis(None)
axis.set_minor_locator(minor_locator)

def _break_share_link(ax, grouper):
siblings = grouper.get_siblings(ax)
Expand All @@ -1607,11 +1615,11 @@ def _break_share_link(ax, grouper):
self.delaxes(ax)
last_ax = _break_share_link(ax, ax._shared_y_axes)
if last_ax is not None:
_reset_loc_form(last_ax.yaxis)
_reset_tickers(last_ax.yaxis)

last_ax = _break_share_link(ax, ax._shared_x_axes)
if last_ax is not None:
_reset_loc_form(last_ax.xaxis)
_reset_tickers(last_ax.xaxis)

def clf(self, keep_observers=False):
"""
Expand Down
14 changes: 11 additions & 3 deletions lib/matplotlib/projections/polar.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,14 @@ class _AxisWrapper(object):
def __init__(self, axis):
self._axis = axis

def __eq__(self, other):
# Needed so that assignment, as the locator.axis attribute, of another
# _AxisWrapper wrapping the same axis works.
return self._axis == other._axis

def __hash__(self):
return hash((type(self), *sorted(vars(self).items())))

def get_view_interval(self):
return np.rad2deg(self._axis.get_view_interval())

Expand Down Expand Up @@ -227,10 +235,11 @@ class ThetaLocator(mticker.Locator):
"""
def __init__(self, base):
self.base = base
self.axis = self.base.axis = _AxisWrapper(self.base.axis)
self.set_axis(self.base.axis)

def set_axis(self, axis):
self.axis = _AxisWrapper(axis)
super().set_axis(_AxisWrapper(axis))
self.base.set_axis(None) # Bypass prevention of axis resetting.
self.base.set_axis(self.axis)

def __call__(self):
Expand Down Expand Up @@ -383,7 +392,6 @@ def _wrap_locator_formatter(self):
def cla(self):
super().cla()
self.set_ticks_position('none')
self._wrap_locator_formatter()

def _set_scale(self, value, **kwargs):
super()._set_scale(value, **kwargs)
Expand Down
26 changes: 26 additions & 0 deletions lib/matplotlib/tests/test_ticker.py
Original file line number Diff line number Diff line change
Expand Up @@ -897,3 +897,29 @@ def minorticksubplot(xminor, yminor, i):
minorticksubplot(True, False, 2)
minorticksubplot(False, True, 3)
minorticksubplot(True, True, 4)


def test_multiple_assignment():
fig = plt.figure()

ax = fig.subplots()
fmt = mticker.NullFormatter()
ax.xaxis.set_major_formatter(fmt)
ax.yaxis.set_major_formatter(fmt)
fig.canvas.draw() # No error.
fig.clf()

ax = fig.subplots()
fmt = mticker.ScalarFormatter()
ax.xaxis.set_major_formatter(fmt)
ax.xaxis.set_minor_formatter(fmt)
fig.canvas.draw() # No error.
fig.clf()

ax = fig.subplots()
fmt = mticker.ScalarFormatter()
ax.xaxis.set_major_formatter(fmt)
ax.yaxis.set_major_formatter(fmt)
with pytest.raises(RuntimeError):
fig.canvas.draw()
fig.clf()
38 changes: 34 additions & 4 deletions lib/matplotlib/ticker.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,41 @@ def get_tick_space(self):
return 9


class TickHelper(object):
axis = None
class TickHelper:
# TickHelpers that access their axis attribute can only be assigned to
# one Axis at a time, but we don't know a priori whether they will (e.g.,
# NullFormatter doesn't, but ScalarFormatter does). So keep track of all
# Axises that a TickHelper is assigned to (in a set: a TickHelper could be
# assigned both as major and minor helper on a single axis), but only error
# out after multiple assignment when the attribute is accessed.

def set_axis(self, axis):
self.axis = axis
# As an escape hatch, allow resetting the axis by first setting it to None.

@property
def axis(self):
# We can't set the '_set_axises' attribute in TickHelper.__init__
# (without a deprecation period) because subclasses didn't have to call
# super().__init__ so far so they likely didn't.
set_axises = getattr(self, "_set_axises", set())
if len(set_axises) == 0:
return None
elif len(set_axises) == 1:
axis, = set_axises
return axis
else:
raise RuntimeError(
f"The 'axis' attribute of this {type(self).__name__} object "
f"has been set multiple times, but a {type(self).__name__} "
f"can only be used for one Axis at a time")

@axis.setter
def axis(self, axis):
if not hasattr(self, "_set_axises") or axis is None:
self._set_axises = set()
if axis is not None:
self._set_axises.add(axis)

set_axis = axis.fset

def create_dummy_axis(self, **kwargs):
if self.axis is None:
Expand Down