Skip to content

Commit

Permalink
Privatize axis.converter attribute
Browse files Browse the repository at this point in the history
The replacement is the get/set_converter method.

This includes changes to use the getter and the private setter in the qt
figure options dialog menu.

The choice to use the private setter was a defensive one as the public
setter prevents being called multiple times (though does short circuit
if an identical input is provided, which I think is actually true here,
therefore the public one is probably functional (and a no-op).)

It is not clear to me on analysis how the unit information is or was
lost. A quick test commenting out the two lines which reset
converter/units displayed no obvious detrimental effect to removing
those, suggesting that even if once they were necessary, they may no
longer be.

These lines were last touched in matplotlib#24141, though that really only generalized
the code into a loop rather than copy/pasted x and y behavior.
The original inclusion of resetting was in matplotlib#4909, which indicated that
the dialog reset unit info. AFAICT, that is no longer true, though I
have not rigorously proved that.
  • Loading branch information
ksunden committed Oct 31, 2024
1 parent 4bb8c8d commit 493fbc6
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 28 deletions.
46 changes: 25 additions & 21 deletions lib/matplotlib/axis.py
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,10 @@ class Axis(martist.Artist):
# The class used in _get_tick() to create tick instances. Must either be
# overwritten in subclasses, or subclasses must reimplement _get_tick().
_tick_class = None
converter = _api.deprecate_privatize_attribute(
"3.10",
alternative="get_converter and set_converter methods"
)

def __str__(self):
return "{}({},{})".format(
Expand Down Expand Up @@ -656,7 +660,7 @@ def __init__(self, axes, *, pickradius=15, clear=True):
if clear:
self.clear()
else:
self.converter = None
self._converter = None
self._converter_is_explicit = False
self.units = None

Expand Down Expand Up @@ -887,7 +891,7 @@ def clear(self):
mpl.rcParams['axes.grid.which'] in ('both', 'minor'))
self.reset_ticks()

self.converter = None
self._converter = None
self._converter_is_explicit = False
self.units = None
self.stale = True
Expand Down Expand Up @@ -1740,20 +1744,20 @@ def grid(self, visible=None, which='major', **kwargs):
def update_units(self, data):
"""
Introspect *data* for units converter and update the
``axis.converter`` instance if necessary. Return *True*
``axis.get_converter`` instance if necessary. Return *True*
if *data* is registered for unit conversion.
"""
if not self._converter_is_explicit:
converter = munits.registry.get_converter(data)
else:
converter = self.converter
converter = self._converter

if converter is None:
return False

neednew = self.converter != converter
neednew = self._converter != converter
self._set_converter(converter)
default = self.converter.default_units(data, self)
default = self._converter.default_units(data, self)
if default is not None and self.units is None:
self.set_units(default)

Expand All @@ -1767,10 +1771,10 @@ def _update_axisinfo(self):
Check the axis converter for the stored units to see if the
axis info needs to be updated.
"""
if self.converter is None:
if self._converter is None:
return

info = self.converter.axisinfo(self.units, self)
info = self._converter.axisinfo(self.units, self)

if info is None:
return
Expand All @@ -1797,20 +1801,20 @@ def _update_axisinfo(self):
self.set_default_intervals()

def have_units(self):
return self.converter is not None or self.units is not None
return self._converter is not None or self.units is not None

def convert_units(self, x):
# If x is natively supported by Matplotlib, doesn't need converting
if munits._is_natively_supported(x):
return x

if self.converter is None:
if self._converter is None:
self._set_converter(munits.registry.get_converter(x))

if self.converter is None:
if self._converter is None:
return x
try:
ret = self.converter.convert(x, self.units, self)
ret = self._converter.convert(x, self.units, self)
except Exception as e:
raise munits.ConversionError('Failed to convert value(s) to axis '
f'units: {x!r}') from e
Expand All @@ -1824,7 +1828,7 @@ def get_converter(self):
-------
`~matplotlib.units.ConversionInterface` or None
"""
return self.converter
return self._converter

def set_converter(self, converter):
"""
Expand All @@ -1838,16 +1842,16 @@ def set_converter(self, converter):
self._converter_is_explicit = True

def _set_converter(self, converter):
if self.converter == converter:
if self._converter == converter:
return
if self._converter_is_explicit:
raise RuntimeError("Axis already has an explicit converter set")
elif self.converter is not None:
elif self._converter is not None:
_api.warn_external(
"This axis already has an converter set and "
"This axis already has a converter set and "
"is updating to a potentially incompatible converter"
)
self.converter = converter
self._converter = converter

def set_units(self, u):
"""
Expand Down Expand Up @@ -2568,8 +2572,8 @@ def set_default_intervals(self):
# not changed the view:
if (not self.axes.dataLim.mutatedx() and
not self.axes.viewLim.mutatedx()):
if self.converter is not None:
info = self.converter.axisinfo(self.units, self)
if self._converter is not None:
info = self._converter.axisinfo(self.units, self)
if info.default_limits is not None:
xmin, xmax = self.convert_units(info.default_limits)
self.axes.viewLim.intervalx = xmin, xmax
Expand Down Expand Up @@ -2798,8 +2802,8 @@ def set_default_intervals(self):
# not changed the view:
if (not self.axes.dataLim.mutatedy() and
not self.axes.viewLim.mutatedy()):
if self.converter is not None:
info = self.converter.axisinfo(self.units, self)
if self._converter is not None:
info = self._converter.axisinfo(self.units, self)
if info.default_limits is not None:
ymin, ymax = self.convert_units(info.default_limits)
self.axes.viewLim.intervaly = ymin, ymax
Expand Down
6 changes: 3 additions & 3 deletions lib/matplotlib/backends/qt_editor/figureoptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def convert_limits(lim, converter):
axis_map = axes._axis_map
axis_limits = {
name: tuple(convert_limits(
getattr(axes, f'get_{name}lim')(), axis.converter
getattr(axes, f'get_{name}lim')(), axis.get_converter()
))
for name, axis in axis_map.items()
}
Expand All @@ -66,7 +66,7 @@ def convert_limits(lim, converter):

# Save the converter and unit data
axis_converter = {
name: axis.converter
name: axis.get_converter()
for name, axis in axis_map.items()
}
axis_units = {
Expand Down Expand Up @@ -209,7 +209,7 @@ def apply_callback(data):
axis.set_label_text(axis_label)

# Restore the unit data
axis.converter = axis_converter[name]
axis._set_converter(axis_converter[name])
axis.set_units(axis_units[name])

# Set / Curves
Expand Down
6 changes: 4 additions & 2 deletions lib/matplotlib/tests/test_dates.py
Original file line number Diff line number Diff line change
Expand Up @@ -668,10 +668,12 @@ def test_concise_converter_stays():
fig, ax = plt.subplots()
ax.plot(x, y)
# Bypass Switchable date converter
ax.xaxis.converter = conv = mdates.ConciseDateConverter()
conv = mdates.ConciseDateConverter()
with pytest.warns(UserWarning, match="already has a converter"):
ax.xaxis.set_converter(conv)
assert ax.xaxis.units is None
ax.set_xlim(*x)
assert ax.xaxis.converter == conv
assert ax.xaxis.get_converter() == conv


def test_offset_changes():
Expand Down
4 changes: 2 additions & 2 deletions lib/matplotlib/tests/test_units.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,10 @@ def test_explicit_converter():
# Explicit is set
fig1, ax1 = plt.subplots()
ax1.xaxis.set_converter(str_cat_converter)
assert ax1.xaxis.converter == str_cat_converter
assert ax1.xaxis.get_converter() == str_cat_converter
# Explicit not overridden by implicit
ax1.plot(d1.keys(), d1.values())
assert ax1.xaxis.converter == str_cat_converter
assert ax1.xaxis.get_converter() == str_cat_converter
# No error when called twice with equivalent input
ax1.xaxis.set_converter(str_cat_converter)
# Error when explicit called twice
Expand Down

0 comments on commit 493fbc6

Please sign in to comment.