Enforce convert() and un_convert()

dstansby · dstansby · commit 35a73a25d879 · 2019-09-27T12:39:44.000+01:00
diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py
@@ -97,7 +97,10 @@ def test_matshow():
                    ])
 def test_formatter_ticker():
     import matplotlib.testing.jpl_units as units
-    units.register()
+    # Catch warnings thrown whilst jpl unit converters don't have an
+    # un_convert() method
+    with pytest.warns(Warning, match='does not define an un_convert'):
+        units.register()
 
     # This should affect the tick size.  (Tests issue #543)
     matplotlib.rcParams['lines.markeredgewidth'] = 30
@@ -486,7 +489,10 @@ def test_polar_alignment():
 def test_fill_units():
     from datetime import datetime
     import matplotlib.testing.jpl_units as units
-    units.register()
+    # Catch warnings thrown whilst jpl unit converters don't have an
+    # un_convert() method
+    with pytest.warns(Warning, match='does not define an un_convert'):
+        units.register()
 
     # generate some data
     t = units.Epoch("ET", dt=datetime(2009, 4, 27))
@@ -654,7 +660,10 @@ def test_polar_wrap(fig_test, fig_ref):
 @check_figures_equal()
 def test_polar_units_1(fig_test, fig_ref):
     import matplotlib.testing.jpl_units as units
-    units.register()
+    # Catch warnings thrown whilst jpl unit converters don't have an
+    # un_convert() method
+    with pytest.warns(Warning, match='does not define an un_convert'):
+        units.register()
     xs = [30.0, 45.0, 60.0, 90.0]
     ys = [1.0, 2.0, 3.0, 4.0]
 
@@ -838,7 +847,10 @@ def test_aitoff_proj():
 def test_axvspan_epoch():
     from datetime import datetime
     import matplotlib.testing.jpl_units as units
-    units.register()
+    # Catch warnings thrown whilst jpl unit converters don't have an
+    # un_convert() method
+    with pytest.warns(Warning, match='does not define an un_convert'):
+        units.register()
 
     # generate some data
     t0 = units.Epoch("ET", dt=datetime(2009, 1, 20))
@@ -854,7 +866,10 @@ def test_axvspan_epoch():
 def test_axhspan_epoch():
     from datetime import datetime
     import matplotlib.testing.jpl_units as units
-    units.register()
+    # Catch warnings thrown whilst jpl unit converters don't have an
+    # un_convert() method
+    with pytest.warns(Warning, match='does not define an un_convert'):
+        units.register()
 
     # generate some data
     t0 = units.Epoch("ET", dt=datetime(2009, 1, 20))
diff --git a/lib/matplotlib/tests/test_units.py b/lib/matplotlib/tests/test_units.py
@@ -3,6 +3,7 @@
 from unittest.mock import MagicMock
 
 import matplotlib.pyplot as plt
+from matplotlib import cbook
 from matplotlib.testing.decorators import check_figures_equal, image_comparison
 import matplotlib.units as munits
 import numpy as np
@@ -56,6 +57,9 @@ def convert(value, unit, axis):
         else:
             return Quantity(value, axis.get_units()).to(unit).magnitude
 
+    def un_convert(value, unit, axis):
+        return Quantitfy(value, unit)
+
     def default_units(value, axis):
         if hasattr(value, 'units'):
             return value.units
@@ -68,6 +72,7 @@ def default_units(value, axis):
     qc.convert = MagicMock(side_effect=convert)
     qc.axisinfo = MagicMock(side_effect=lambda u, a: munits.AxisInfo(label=u))
     qc.default_units = MagicMock(side_effect=default_units)
+    qc.un_convert = MagicMock(side_effect=un_convert)
     return qc
 
 
@@ -124,7 +129,10 @@ def test_empty_set_limits_with_units(quantity_converter):
                   savefig_kwarg={'dpi': 120}, style='mpl20')
 def test_jpl_bar_units():
     import matplotlib.testing.jpl_units as units
-    units.register()
+    # Catch warnings thrown whilst jpl unit converters don't have an
+    # un_convert() method
+    with pytest.warns(Warning, match='does not define an un_convert'):
+        units.register()
 
     day = units.Duration("ET", 24.0 * 60.0 * 60.0)
     x = [0*units.km, 1*units.km, 2*units.km]
@@ -140,7 +148,10 @@ def test_jpl_bar_units():
                   savefig_kwarg={'dpi': 120}, style='mpl20')
 def test_jpl_barh_units():
     import matplotlib.testing.jpl_units as units
-    units.register()
+    # Catch warnings thrown whilst jpl unit converters don't have an
+    # un_convert() method
+    with pytest.warns(Warning, match='does not define an un_convert'):
+        units.register()
 
     day = units.Duration("ET", 24.0 * 60.0 * 60.0)
     x = [0*units.km, 1*units.km, 2*units.km]
@@ -175,3 +186,17 @@ class subdate(datetime):
 
     fig_test.subplots().plot(subdate(2000, 1, 1), 0, "o")
     fig_ref.subplots().plot(datetime(2000, 1, 1), 0, "o")
+
+
+def test_no_conveter_warnings():
+    class Converter(munits.ConversionInterface):
+        pass
+
+    # Check that a converter without a manuallly defined convert() method
+    # warns
+    with pytest.warns(cbook.deprecation.MatplotlibDeprecationWarning):
+        Converter.convert(0, 0, 0)
+
+    # Check that manually defining a conveter doesn't warn
+    Converter.convert = lambda obj, unit, axis: obj
+    Converter.convert(0, 0, 0)
diff --git a/lib/matplotlib/units.py b/lib/matplotlib/units.py
@@ -22,6 +22,11 @@ def convert(value, unit, axis):
             'Convert a datetime value to a scalar or array'
             return dates.date2num(value)
 
+        @staticmethod
+        def un_convert(value, unit, axis):
+            'Convert a float back to a datetime value'
+            return dates.num2date(value)
+
         @staticmethod
         def axisinfo(unit, axis):
             'Return major and minor tick locators and formatters'
@@ -44,6 +49,7 @@ def default_units(x, axis):
 
 from decimal import Decimal
 from numbers import Number
+import warnings
 
 import numpy as np
 from numpy import ma
@@ -127,6 +133,7 @@ def default_units(x, axis):
         """
         return None
 
+    # Make this an abstractmethod in 3.5
     @staticmethod
     def convert(obj, unit, axis):
         """
@@ -135,15 +142,25 @@ def convert(obj, unit, axis):
         If *obj* is a sequence, return the converted sequence.  The output must
         be a sequence of scalars that can be used by the numpy array layer.
         """
+        cbook.warn_deprecated(
+            '3.3',
+            message=('Using the default "does nothing" convert() method for '
+                     'Matplotlib ConversionInterface converters is deprecated '
+                     'and will raise an error in version 3.5. '
+                     'Please manually override convert().'))
         return obj
 
+    # Uncomment this in version 3.5 to enforce an un_convert() method
+    '''
     @staticmethod
+    @abc.abstractmethod
     def un_convert(data, unit, axis):
         """
         Convert data that has already been converted back to its original
         value.
         """
-        return data
+        pass
+    '''
 
     @staticmethod
     def is_numlike(x):
@@ -188,6 +205,14 @@ def convert(value, unit, axis):
                 converter = ma.asarray
             return converter(value, dtype=np.float)
 
+    @staticmethod
+    def un_convert(value, unit, axis):
+        """
+        Un-convert from floats to Decimals.
+        """
+        return Decimal(value)
+
+
     @staticmethod
     def axisinfo(unit, axis):
         # Since Decimal is a kind of Number, don't need specific axisinfo.
@@ -202,6 +227,14 @@ def default_units(x, axis):
 class Registry(dict):
     """Register types with conversion interface."""
 
+    def __setitem__(self, cls, converter):
+        if not hasattr(converter, 'un_convert'):
+            warnings.warn(
+                f'{converter.__class__.__name__} does not define an '
+                'un_convert() method. From Matplotlib 3.5 this will be '
+                'required, and if not present will raise an error.')
+        super().__setitem__(cls, converter)
+
     def get_converter(self, x):
         """Get the converter interface instance for *x*, or None."""
         if hasattr(x, "values"):
