Repository: matplotlib/matplotlib
Pull request: Add explicit converter setting to Axis
Closes #19229

<!--
Thank you so much for your PR!  To help us review your contribution, please check
out the development guide https://matplotlib.org/devdocs/devel/index.html
-->

## PR summary
<!-- Please provide at least 1-2 sentences describing the pull request in detail
(Why is this change required?  What problem does it solve?) and link to relevant
issues and PRs.

Also please summarize the changes in the title, for example "Raise ValueError on
non-numeric input to set_xlim" and avoid non-descriptive titles such as "Addresses
issue #8576".
-->
Adds an API to explicitly set the converter for an Axis.

Once explicitly set, the implicit machinery will not override the converter.
Previously (and still, if not explicitly set), the converter would be reset each time new data was added, possibly setting to something incompatible with previously provided data.

You cannot reset the converter once explicitly provided (except by clearing the axis entirely).
A warning is provided if implicitly set and being set to something different.


## PR checklist
<!-- Please mark any checkboxes that do not apply to this PR as [N/A].-->

- [ ] "closes #0000" is in the body of the PR description to [link the related issue](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
- [ ] new and changed code is [tested](https://matplotlib.org/devdocs/devel/testing.html)
- [ ] *Plotting related* features are demonstrated in an [example](https://matplotlib.org/devdocs/devel/document.html#write-examples-and-tutorials)
- [ ] *New Features* and *API Changes* are noted with a [directive and release note](https://matplotlib.org/devdocs/devel/api_changes.html#announce-changes-deprecations-and-new-features)
- [ ] Documentation complies with [general](https://matplotlib.org/devdocs/devel/document.html#write-rest-pages) and [docstring](https://matplotlib.org/devdocs/devel/document.html#write-docstrings) guidelines

<!--We understand that PRs can sometimes be overwhelming, especially as the
reviews start coming in.  Please let us know if the reviews are unclear or
the recommended next step seems overly demanding, if you would like help in
addressing a reviewer's comments, or if you have been waiting too long to hear
back on your PR.-->


Problem statement:
Add public API for setting an axis unit converter
### Problem

I would like to define and use a custom unit converter class. The existing `mpl.units.registry` system is type-based, which does not work for my usecase. I need my converter to generalize over types. But I also know exactly which (existing) axis objects should use the converter.

The closest I can get in associating a custom converter with an existing axis, following the code in `axis.update_units` is

```python
ax = plt.figure().subplots()
ax.xaxis.converter = CustomConverter()
ax.xaxis._update_axisinfo()
ax.xaxis.stale = True
```

### Proposed Solution

A public `axis.set_converter` API would solve this problem, e.g.

```python
ax = plt.figure().subplots()
ax.set_converter(CustomConverter())
```

The idea is that this would basically do what `Axis.update_units` does, but consuming a `ConversionInterface` rather than consuming data and then getting a converter from the registry.

### Additional context and prior art

The other thing I would mention is that I could probably work-around this problem if the registry lookup were based on the container `dtype` rather than the type of the individual elements (as is currently the case). But that seems to complexify the existing API, whereas the this proposal would add an orthogonal API, and it is probably better for that reason.

Hints from issue discussion:
This would not be enough as https://github.com/matplotlib/matplotlib/pull/10206 added a cut-out (which was refactored a bit in https://github.com/matplotlib/matplotlib/pull/13738 and https://github.com/matplotlib/matplotlib/pull/14363) to not try and convert numbers.

In addition to adding `set_converter` would would have to extend the public API of the converters to at the instance level bail out of trying to do the conversion.  It looks like we could do this by over-riding `is_numlike` on sub-classes, but then we can not defer the "check if we need to predict a converter" step (which has been in the code base from 84ede567056bde8411446146854b86cdb2ff5ae3 ) 
I think we could remove the cutout and make dealing with numbers the prerogative of the converter?   
FYI I've decided to solve this in seaborn by internally coercing data to strings in situations where it should receive "categorical" treatment. I I think, for now at least, I don't need any changes in matplotlib to support this particular use-case. I'm going to close the issue, but if you think this is a good idea independently of my narrow problem, feel free to re-open.
I think we should keep this open for the upcoming unit work.  
This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear.  Thanks for your help!

Commit message: Add explicit converter setting to Axis

Closes #19229

Patch to review:
diff --git a/lib/matplotlib/axis.py b/lib/matplotlib/axis.py
index 8e612bd8c702..3b5de64bbcec 100644
--- a/lib/matplotlib/axis.py
+++ b/lib/matplotlib/axis.py
@@ -657,6 +657,7 @@ def __init__(self, axes, *, pickradius=15, clear=True):
             self.clear()
         else:
             self.converter = None
+            self._explicit_converter = False
             self.units = None
 
         self._autoscale_on = True
@@ -887,6 +888,7 @@ def clear(self):
         self.reset_ticks()
 
         self.converter = None
+        self._explicit_converter = False
         self.units = None
         self.stale = True
 
@@ -1741,12 +1743,16 @@ def update_units(self, data):
         ``axis.converter`` instance if necessary. Return *True*
         if *data* is registered for unit conversion.
         """
-        converter = munits.registry.get_converter(data)
+        if not self._explicit_converter:
+            converter = munits.registry.get_converter(data)
+        else:
+            converter = self.converter
+
         if converter is None:
             return False
 
         neednew = self.converter != converter
-        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)
@@ -1799,7 +1805,7 @@ def convert_units(self, x):
             return x
 
         if self.converter is None:
-            self.converter = munits.registry.get_converter(x)
+            self._set_converter(munits.registry.get_converter(x))
 
         if self.converter is None:
             return x
@@ -1810,6 +1816,22 @@ def convert_units(self, x):
                                          f'units: {x!r}') from e
         return ret
 
+    def set_converter(self, converter):
+        self._set_converter(converter)
+        self._explicit_converter = True
+
+    def _set_converter(self, converter):
+        if self.converter == converter:
+            return
+        if self._explicit_converter:
+            raise RuntimeError("Axis already has an explicit converter set")
+        elif self.converter is not None:
+            _api.warn_external(
+                "This axis already has an converter set and "
+                "is updating to a potentially incompatible converter"
+            )
+        self.converter = converter
+
     def set_units(self, u):
         """
         Set the units for axis.
diff --git a/lib/matplotlib/tests/test_units.py b/lib/matplotlib/tests/test_units.py
index ae6372fea1e1..370b5804c485 100644
--- a/lib/matplotlib/tests/test_units.py
+++ b/lib/matplotlib/tests/test_units.py
@@ -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
 
@@ -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.converter == str_cat_converter
+    # Explicit not overridden by implicit
+    ax1.plot(d1.keys(), d1.values())
+    assert ax1.xaxis.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()


Code review comments:
diff_hunk: @@ -657,6 +657,7 @@ def __init__(self, axes, *, pickradius=15, clear=True):
             self.clear()
         else:
             self.converter = None
+            self._explicit_converter = False
path: lib/matplotlib/axis.py
text: @user1:
Slight perference for
```suggestion
            self._converter_is_explicit = False
```
This makes it clear that the content is a bool. `_explicit_converter` could be understood to contain an explicit converter instance.