Unidata / MetPy

MetPy is a collection of tools in Python for reading, visualizing and performing calculations with weather data.
https://unidata.github.io/MetPy/
BSD 3-Clause "New" or "Revised" License
1.21k stars 408 forks source link

Failing test on hodograph #832

Closed jrleeman closed 3 years ago

jrleeman commented 6 years ago

The test test_united_hodograph_range was added when addressing #805, but was marked as an expected fail due to an issue in matplotlib. Once the issue with set_ylim is addressed in matplotlib, we'll need to make sure this test is passing and not an expected failure.

akrherz commented 5 years ago

I was looking into this and wondering which matplotlib bug is being referenced here, is it matplotlib issue 9713? I added this hack patch, which got the test to no longer bomb, but am unsure yet of implications.

diff --git a/metpy/units.py b/metpy/units.py
index 66cac9e..ec3c3af 100644
--- a/metpy/units.py
+++ b/metpy/units.py
@@ -348,6 +348,8 @@ except (AttributeError, RuntimeError):  # Pint's not available, try to enable ou
         def _convert_value(self, value, unit, axis):
             """Handle converting using attached unit or falling back to axis units."""
             if hasattr(value, 'units'):
+                if unit is None:
+                    return value.magnitude
                 return value.to(unit).magnitude
             else:
                 return self._reg.Quantity(value, axis.get_units()).to(unit).magnitude
dopplershift commented 5 years ago

So apparently I never opened the issue on the matplotlib repo. 🐑 So thanks for digging on this and making me actually submit a fix to matplotlib.

Your fix removes the traceback, but is unlikely to play right across using different units. It's a simple enough fix in matplotlib (see matplotlib/matplotlib#12468); should see the next 3.0 bug fix release.

zbruick commented 4 years ago

Just looked at this test again - still failing. Looks like it might be the same issue but with set_xlim?