Improve error messages for unit conversion by timhoffm · Pull Request #13005 · matplotlib/matplotlib

timhoffm · 2018-12-17T15:51:51Z

PR Summary

Fixes #12990. The call stack is a bit lengthy, but that's because the error happens several levels down and at least the reason gets clear.

Assigning categoricals to floats.

from matplotlib import pyplot as plt
xs = [1, 2, 3]
ys = [50, 10, 20]
ticks = ['banana', 'apple', 'pear']
plt.bar(xs, ys)
plt.xticks(ticks)

results in

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
~/dev/matplotlib/lib/matplotlib/axis.py in convert_units(self, x)
   1489         try:
-> 1490             ret = self.converter.convert(x, self.units, self)
   1491         except Exception as e:

~/dev/matplotlib/lib/matplotlib/category.py in convert(value, unit, axis)
     45             raise ValueError(
---> 46                 'Missing unit for StrCategoryConverter. This might be caused'
     47                 'by unintendedly mixing categorical and numeric data.')

ValueError: Missing unit for StrCategoryConverter. This might be causedby unintendedly mixing categorical and numeric data.

The above exception was the direct cause of the following exception:

ConversionError                           Traceback (most recent call last)
<ipython-input-2-8d8628e715b5> in <module>()
      4 ticks = ['banana', 'apple', 'pear']
      5 plt.bar(xs, ys)
----> 6 plt.xticks(ticks)

~/dev/matplotlib/lib/matplotlib/pyplot.py in xticks(ticks, labels, **kwargs)
   1518         labels = ax.get_xticklabels()
   1519     elif labels is None:
-> 1520         locs = ax.set_xticks(ticks)
   1521         labels = ax.get_xticklabels()
   1522     else:

~/dev/matplotlib/lib/matplotlib/axes/_base.py in set_xticks(self, ticks, minor)
   3303             Default is ``False``.
   3304         """
-> 3305         ret = self.xaxis.set_ticks(ticks, minor=minor)
   3306         self.stale = True
   3307         return ret

~/dev/matplotlib/lib/matplotlib/axis.py in set_ticks(self, ticks, minor)
   1689         """
   1690         # XXX if the user changes units, the information will be lost here
-> 1691         ticks = self.convert_units(ticks)
   1692         if len(ticks) > 1:
   1693             xleft, xright = self.get_view_interval()

~/dev/matplotlib/lib/matplotlib/axis.py in convert_units(self, x)
   1491         except Exception as e:
   1492             raise munits.ConversionError('Failed to convert value(s) to axis '
-> 1493                                          'units: %r' % x) from e
   1494         return ret
   1495 

ConversionError: Failed to convert value(s) to axis units: ['banana', 'apple', 'pear']

Assigning categoricals to dates.

from matplotlib import pyplot as plt
xs = [datetime(2015, 1, 1), datetime(2016, 1, 1), datetime(2017, 1, 1)]
ys = [50, 10, 20]
ticks = ['banana', 'apple', 'pear']
plt.bar(xs, ys)
plt.xticks(ticks)

results in

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
~/dev/matplotlib/lib/matplotlib/axis.py in convert_units(self, x)
   1489         try:
-> 1490             ret = self.converter.convert(x, self.units, self)
   1491         except Exception as e:

~/dev/matplotlib/lib/matplotlib/dates.py in convert(value, unit, axis)
   1805         """
-> 1806         return date2num(value)
   1807 

~/dev/matplotlib/lib/matplotlib/dates.py in date2num(d)
    421             return d
--> 422         return _to_ordinalf_np_vectorized(d)
    423 

~/miniconda3/envs/mpl/lib/python3.6/site-packages/numpy/lib/function_base.py in __call__(self, *args, **kwargs)
   2754 
-> 2755         return self._vectorize_call(func=func, args=vargs)
   2756 

~/miniconda3/envs/mpl/lib/python3.6/site-packages/numpy/lib/function_base.py in _vectorize_call(self, func, args)
   2824         else:
-> 2825             ufunc, otypes = self._get_ufunc_and_otypes(func=func, args=args)
   2826 

~/miniconda3/envs/mpl/lib/python3.6/site-packages/numpy/lib/function_base.py in _get_ufunc_and_otypes(self, func, args)
   2784             inputs = [arg.flat[0] for arg in args]
-> 2785             outputs = func(*inputs)
   2786 

~/dev/matplotlib/lib/matplotlib/dates.py in _to_ordinalf(dt)
    225 
--> 226     base = float(dt.toordinal())
    227 

AttributeError: 'numpy.str_' object has no attribute 'toordinal'

The above exception was the direct cause of the following exception:

ConversionError                           Traceback (most recent call last)
<ipython-input-3-e7444cbe73e7> in <module>()
      4 ticks = ['banana', 'apple', 'pear']
      5 plt.bar(xs, ys)
----> 6 plt.xticks(ticks)

~/dev/matplotlib/lib/matplotlib/pyplot.py in xticks(ticks, labels, **kwargs)
   1518         labels = ax.get_xticklabels()
   1519     elif labels is None:
-> 1520         locs = ax.set_xticks(ticks)
   1521         labels = ax.get_xticklabels()
   1522     else:

~/dev/matplotlib/lib/matplotlib/axes/_base.py in set_xticks(self, ticks, minor)
   3303             Default is ``False``.
   3304         """
-> 3305         ret = self.xaxis.set_ticks(ticks, minor=minor)
   3306         self.stale = True
   3307         return ret

~/dev/matplotlib/lib/matplotlib/axis.py in set_ticks(self, ticks, minor)
   1689         """
   1690         # XXX if the user changes units, the information will be lost here
-> 1691         ticks = self.convert_units(ticks)
   1692         if len(ticks) > 1:
   1693             xleft, xright = self.get_view_interval()

~/dev/matplotlib/lib/matplotlib/axis.py in convert_units(self, x)
   1491         except Exception as e:
   1492             raise munits.ConversionError('Failed to convert value(s) to axis '
-> 1493                                          'units: %r' % x) from e
   1494         return ret
   1495 

ConversionError: Failed to convert value(s) to axis units: ['banana', 'apple', 'pear']

story645 · 2018-12-17T17:43:09Z

        """
+        if unit is None:
+            raise ValueError(
+                'Missing unit for StrCategoryConverter. This might be caused'


So what's missing here is the mapping from category to float. Worried about using unit in a user facing error because unit is a kwarg that doesn't have to be passed in for categoricals to work.

Worried about using unit in a user facing error because unit is a kwarg that doesn't have to be passed in for categoricals to work.

That's why I'm casting the error message to "Failed to convert..." using raise ... from.

So what's missing here is the mapping from category to float.

Not sure what that means. Moving the example from pyplot to ax to make it a little more obvious:

ax = plt.gca() ax.bar([1, 2, 3], [50, 10, 20]) ax.set_xticks(['banana', 'apple', 'pear'])

We are trying to string positions (not labels) to a float axis. IMHO there's nothing missing here and this should error out. Ideally, the error message would be something like "Attemtping to set categorical ticks to a non-categorical axis".

But there's no good place to raise that.

The original stacktrace before this PR was:

--------------------------------------------------------------------------- AttributeError Traceback (most recent call last) <ipython-input-2-13c713392566> in <module>() 2 ax = plt.gca() 3 ax.bar([1, 2, 3], [50, 10, 20]) ----> 4 ax.set_xticks(['banana', 'apple', 'pear']) ~/dev/matplotlib/lib/matplotlib/axes/_base.py in set_xticks(self, ticks, minor) 3303 Default is ``False``. 3304 """ -> 3305 ret = self.xaxis.set_ticks(ticks, minor=minor) 3306 self.stale = True 3307 return ret ~/dev/matplotlib/lib/matplotlib/axis.py in set_ticks(self, ticks, minor) 1686 """ 1687 # XXX if the user changes units, the information will be lost here -> 1688 ticks = self.convert_units(ticks) 1689 if len(ticks) > 1: 1690 xleft, xright = self.get_view_interval() ~/dev/matplotlib/lib/matplotlib/axis.py in convert_units(self, x) 1488 return x 1489 -> 1490 ret = self.converter.convert(x, self.units, self) 1491 return ret 1492 ~/dev/matplotlib/lib/matplotlib/category.py in convert(value, unit, axis) 51 52 # force an update so it also does type checking ---> 53 unit.update(values) 54 55 str2idx = np.vectorize(unit._mapping.__getitem__, AttributeError: 'NoneType' object has no attribute 'update'

So, you can raise

in set_ticks(self, ticks, minor)
Advantage: You know you are setting ticks, so you could issue "Attemtping to set categorical ticks to a non-categorical axis".
Disadvantage: You need to explicitly check ticks and the unit of Axis.

in convert_units(self, x)
Advantage: You can investigate self.units and generate a more appropriate message related to categoricals.
Disadvantage: You don't know that you are setting ticks here. You would have to explicitly check which converter you are using.

in convert(value, unit, axis)
Advantage: You know that unit must not be None so the check is trivial.
Disadvantage: You don't know anything about categorical axes anymore and therefore can only state that unit is missing and add a guess why that might be the case.

In that situation I decided to raise on 3. add add context information in 2. using the raise .. from.

I'm not against the error, I worry that the specific word unit will lead users down a google rabbit hole that's misguided. Basically, there's an xunit/yunit kwarg:
https://matplotlib.org/gallery/units/bar_demo2.html#sphx-glr-gallery-units-bar-demo2-py

axs[1, 0].bar(cms, cms, bottom=bottom, width=width, xunits=inch, yunits=cm)

So I worry an error message with the word unit might make users think the fix is to pass in something, when really the issue is that the axis input data is not a string so the axis is not registered as string units. I wonder if the following is enough:

'Axis does not support StrCategoryConvertor; axis data values must be strings'

Like you, I want to help people catch that the problem is the mismatch between the plot data and the tick data.

Rephrased to

raise ValueError( 'Missing category information for StrCategoryConverter; ' 'this might be caused by unintendedly mixing categorical and ' 'numeric data')

I don't want to mention an Axis as we don't know the call context here. In the end, this is also why we can only guess the reason for the failure but not make a clear statement what to change.

anntzer · 2018-12-17T18:07:06Z

 from matplotlib import cbook


+class ConversionError(ValueError):


this is a bit bikesheddy but it looks more like a TypeError than a ValueError to me.

anntzer · 2018-12-17T18:13:07Z

+        if unit is None:
+            raise ValueError(
+                'Missing unit for StrCategoryConverter. This might be caused'
+                'by unintendedly mixing categorical and numeric data.')


Minor preference for error messages that are single sentence and without a final dot, as that's what we mostly have in the codebase (so replace the middle dot by a semicolon).
Also a missing space after "caused".

timhoffm · 2019-01-10T17:53:54Z

Travis CI failure is due to #13137 and unrelated to this PR.

By me, this is ready to go in.

anntzer · 2019-01-11T14:24:16Z

    @staticmethod
    def convert(value, unit, axis):
-        """Converts strings in value to floats using
+        """Convert strings in value to floats using


Just inherit the docstring from units.ConversionInterface perhaps? (and possibly edit that docstring).

I don't think we can reasonably use an inherited docstring here. Using the conversion interface for string-index mapping is a bit of a stretch and justifies explicit description.

type of value is a string, which is rather the exception for ConversionInterface

mark unit as UnitData

note that axis is unused.

anntzer

anyone can merge (including tim) when ci passes.

timhoffm · 2019-01-12T19:58:39Z

Self-merging based on the reviews. The Travis CI failure is still due to #13137 (unrelated to this PR).

tacaswell · 2019-01-14T04:04:56Z

Re-milestoned to 3.1 so we do not have to deal with backporting the f string and because there is a subtle API change (the type of the raise exception) in this.

timhoffm added this to the v3.0.3 milestone Dec 17, 2018

story645 reviewed Dec 17, 2018

View reviewed changes

anntzer reviewed Dec 17, 2018

View reviewed changes

timhoffm force-pushed the unit-conversion-error-message branch from 387c345 to 83a3631 Compare December 17, 2018 19:21

timhoffm added the API: argument checking Validation of acceptable argument values label Dec 17, 2018

dstansby approved these changes Dec 18, 2018

View reviewed changes

timhoffm force-pushed the unit-conversion-error-message branch 4 times, most recently from 4cebc36 to e38410c Compare January 6, 2019 22:56

timhoffm added the status: needs review label Jan 10, 2019

anntzer reviewed Jan 11, 2019

View reviewed changes

Comment thread lib/matplotlib/category.py Resolved

anntzer reviewed Jan 11, 2019

View reviewed changes

Comment thread lib/matplotlib/axis.py Resolved

anntzer removed the status: needs review label Jan 11, 2019

Improve error messages for unit conversion

c9a0ac7

timhoffm force-pushed the unit-conversion-error-message branch from e38410c to c9a0ac7 Compare January 12, 2019 16:14

anntzer approved these changes Jan 12, 2019

View reviewed changes

timhoffm merged commit c8933e0 into matplotlib:master Jan 12, 2019

timhoffm deleted the unit-conversion-error-message branch January 12, 2019 19:58

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jan 12, 2019

Backport PR matplotlib#13005: Improve error messages for unit conversion

c38d32d

meeseeksmachine mentioned this pull request Jan 12, 2019

Backport PR #13005 on branch v3.0.x (Improve error messages for unit conversion) #13168

Closed

tacaswell modified the milestones: v3.0.3, v3.1 Jan 14, 2019

pllim mentioned this pull request Jan 15, 2019

TST: Fix viz test failure due to MPL error refactoring astropy/astropy#8352

Merged

QuLogic mentioned this pull request May 2, 2023

Error Handling of Non-Ints/Floats for postion of xticks #10147

Closed

		from matplotlib import cbook


		class ConversionError(ValueError):

Conversation

timhoffm commented Dec 17, 2018

PR Summary

Assigning categoricals to floats.

Assigning categoricals to dates.

Uh oh!

story645 Dec 17, 2018

Choose a reason for hiding this comment

Uh oh!

timhoffm Dec 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

story645 Dec 17, 2018

Choose a reason for hiding this comment

Uh oh!

timhoffm Jan 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anntzer Dec 17, 2018

Choose a reason for hiding this comment

Uh oh!

anntzer Dec 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timhoffm commented Jan 10, 2019

Uh oh!

anntzer Jan 11, 2019

Choose a reason for hiding this comment

Uh oh!

timhoffm Jan 12, 2019

Choose a reason for hiding this comment

Uh oh!

anntzer Jan 12, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

anntzer left a comment

Choose a reason for hiding this comment

Uh oh!

timhoffm commented Jan 12, 2019

Uh oh!

tacaswell commented Jan 14, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

timhoffm Dec 17, 2018 •

edited

Loading

timhoffm Jan 6, 2019 •

edited

Loading

anntzer Dec 17, 2018 •

edited

Loading