Skip to content

Add explicit converter setting to Axis #28970

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 4 commits into from
Oct 31, 2024
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
2 changes: 2 additions & 0 deletions doc/api/axis_api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ Units
Axis.convert_units
Axis.set_units
Axis.get_units
Axis.set_converter
Axis.get_converter
Axis.update_units


Expand Down
79 changes: 61 additions & 18 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,8 @@ 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

self._autoscale_on = True
Expand Down Expand Up @@ -886,7 +891,8 @@ 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 @@ -1738,16 +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.
"""
converter = munits.registry.get_converter(data)
if not self._converter_is_explicit:
converter = munits.registry.get_converter(data)
else:
converter = self._converter

if converter is None:
return False

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

Expand All @@ -1761,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 @@ -1791,25 +1801,58 @@ 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:
self.converter = munits.registry.get_converter(x)
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
return ret

def get_converter(self):
"""
Get the unit converter for axis.

Returns
-------
`~matplotlib.units.ConversionInterface` or None
"""
return self._converter

def set_converter(self, converter):
"""
Set the unit converter for axis.

Parameters
----------
converter : `~matplotlib.units.ConversionInterface`
"""
self._set_converter(converter)
self._converter_is_explicit = True

def _set_converter(self, 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:
_api.warn_external(
"This axis already has a converter set and "
"is updating to a potentially incompatible converter"
)
self._converter = converter

def set_units(self, u):
"""
Set the units for axis.
Expand Down Expand Up @@ -2529,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 @@ -2759,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
3 changes: 3 additions & 0 deletions lib/matplotlib/axis.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ from matplotlib.text import Text
from matplotlib.ticker import Locator, Formatter
from matplotlib.transforms import Transform, Bbox
from matplotlib.typing import ColorType
from matplotlib.units import ConversionInterface


GRIDLINE_INTERPOLATION_STEPS: int
Expand Down Expand Up @@ -207,6 +208,8 @@ class Axis(martist.Artist):
def update_units(self, data): ...
def have_units(self) -> bool: ...
def convert_units(self, x): ...
def get_converter(self) -> ConversionInterface | None: ...
def set_converter(self, converter: ConversionInterface) -> None: ...
def set_units(self, u) -> None: ...
def get_units(self): ...
def set_label_text(
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 @@
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 @@

# 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 @@
axis.set_label_text(axis_label)

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

Check warning on line 212 in lib/matplotlib/backends/qt_editor/figureoptions.py

View check run for this annotation

Codecov / codecov/patch

lib/matplotlib/backends/qt_editor/figureoptions.py#L212

Added line #L212 was not covered by tests
axis.set_units(axis_units[name])
Comment on lines +212 to 213
Copy link
Member Author

Choose a reason for hiding this comment

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

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 these two lines which reset
converter/units displayed no obvious detrimental effect to removing
them, suggesting that even if once they were necessary, they may no
longer be.

These lines were last touched in #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 #4909, which indicated that
the dialog reset unit info. AFAICT, that is no longer true, though I
have not rigorously proved that.


# 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
28 changes: 27 additions & 1 deletion lib/matplotlib/tests/test_units.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import matplotlib.pyplot as plt
from matplotlib.testing.decorators import check_figures_equal, image_comparison
import matplotlib.units as munits
from matplotlib.category import UnitData
from matplotlib.category import StrCategoryConverter, UnitData
import numpy as np
import pytest

Expand Down Expand Up @@ -236,6 +236,32 @@ def test_shared_axis_categorical():
assert "c" in ax2.xaxis.get_units()._mapping.keys()


def test_explicit_converter():
d1 = {"a": 1, "b": 2}
str_cat_converter = StrCategoryConverter()
str_cat_converter_2 = StrCategoryConverter()

# Explicit is set
fig1, ax1 = plt.subplots()
ax1.xaxis.set_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.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
with pytest.raises(RuntimeError):
ax1.xaxis.set_converter(str_cat_converter_2)

# Warn when implicit overridden
fig2, ax2 = plt.subplots()
ax2.plot(d1.keys(), d1.values())

with pytest.warns():
ax2.xaxis.set_converter(str_cat_converter)


def test_empty_default_limits(quantity_converter):
munits.registry[Quantity] = quantity_converter
fig, ax1 = plt.subplots()
Expand Down
Loading