Repository: matplotlib/matplotlib
Pull request: FIX: (broken)bar(h) math before units
## PR Summary

OK, this works w/ `bar`, `barh`, and `broken_barh`, and I think it even passes the tests....

~EDIT: now works w/ bar, but see issue below with the pdf (and pgf) determinism tests...~

EDIT:  OK, this may not work:   i.e. consider `ax.bar(['a', 'b'], ['c', 'd'])`.  Width defaults to 0.8, so we can't add 0.8 to 'a'.  It works fine for `datetime` and `timedelta` because they have addition defined, but you can't add a float to a string.  

So, we may have to go back to something like `delta = convert_units_delta(x, delta)` and then assume that the converter can do the math.  

Closes #12862

Also closes #11290, though it doesn't address the call for a timedelta formatter, but at least it doesn't crash!  Now time deltas work for both `bar` and `barh`.  

```python
import matplotlib.pyplot as plt
from datetime import datetime, timedelta

lo = datetime(2018, 11, 9, 20, 0, 0)
hi = datetime(2018, 11, 9, 22, 0, 0)

fig, ax = plt.subplots()

ax.set_xlim(lo-timedelta(hours=1), hi)
ax.set_ylim(0, 30)

ax.broken_barh([(lo, timedelta(hours=1))], (10, 20))
plt.show()

```

fails on master.  Now works by making sure that the timedelta math is done before the unit conversion is done.

## PR Checklist

- [ ] Has Pytest style unit tests
- [ ] Code is [Flake 8](http://flake8.pycqa.org/en/latest/) compliant
- [ ] New features are documented, with examples if plot related
- [ ] Documentation is sphinx and numpydoc compliant
- [ ] Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
- [ ] Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

<!--
Thank you so much for your PR!  To help us review your contribution, please
consider the following points:

- A development guide is available at https://matplotlib.org/devdocs/devel/index.html.

- Help with git and github is available at
  https://matplotlib.org/devel/gitwash/development_workflow.html.

- Do not create the PR out of master, but out of a separate branch.

- The PR title should summarize the changes, for example "Raise ValueError on
  non-numeric input to set_xlim".  Avoid non-descriptive titles such as
  "Addresses issue #8576".

- The summary should 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 any relevant issues.

- If you are contributing fixes to docstrings, please pay attention to
  http://matplotlib.org/devel/documenting_mpl.html#formatting.  In particular,
  note the difference between using single backquotes, double backquotes, and
  asterisks in the markup.

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:
ax.bar doesn't work correctly when width is a timedelta64 object
The following example

```python
In [1]: x = np.array(range(100), dtype='M8[D]')

In [2]: ax = plt.subplot(1,1,1)

In [3]: y = np.random.random(100)

In [4]: ax.bar(x, y)
Out[4]: <BarContainer object of 100 artists>
```

works fine, but if I set width to be the diff of the ``datetime64`` array, which gives a ``timedelta64`` array, things break:

```python
In [7]: ax.bar(x[:-1], y[:-1], width=np.diff(x))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-7-f05692d600cb> in <module>()
----> 1 ax.bar(x[:-1], y[:-1], width=np.diff(x))

~/miniconda3/envs/dev/lib/python3.6/site-packages/matplotlib/__init__.py in inner(ax, *args, **kwargs)
   1853                         "the Matplotlib list!)" % (label_namer, func.__name__),
   1854                         RuntimeWarning, stacklevel=2)
-> 1855             return func(ax, *args, **kwargs)
   1856 
   1857         inner.__doc__ = _add_data_doc(inner.__doc__,

~/miniconda3/envs/dev/lib/python3.6/site-packages/matplotlib/axes/_axes.py in bar(self, *args, **kwargs)
   2257         if align == 'center':
   2258             if orientation == 'vertical':
-> 2259                 left = x - width / 2
   2260                 bottom = y
   2261             elif orientation == 'horizontal':

TypeError: ufunc subtract cannot use operands with types dtype('float64') and dtype('<m8[D]')
```

This should however be unambiguous, so it would be nice if it worked.

broken_barh appears not to work with datetime/timedelta objects
<!--To help us understand and resolve your issue, please fill out the form to the best of your ability.-->
<!--You can feel free to delete the sections that do not apply.-->

### Bug report

**Bug summary**

The `Axis.broken_barh` function takes an `xranges` parameter described as "The x-positions and extends of the rectangles". But if I'm plotting datetime values on the axis, it appears impossible to supply an extent value of a type that the function can deal with.

Using `datetime.timedelta` raises an exception `AttributeError: 'datetime.timedelta' object has no attribute 'toordinal'`, as does using an integer. Using a `datetime` value causes the bar to extend too far (which I guess isn't surprising, as `toordinal` on a datetime will *not* give a duration).

**Code for reproduction**

```python
import matplotlib.pyplot as plt
from datetime import datetime, timedelta

lo = datetime.fromisoformat("2018-11-09 20:00")
hi = datetime.fromisoformat("2018-11-09 22:00")

fig, ax = plt.subplots()

ax.set_xlim(lo, hi)
ax.set_ylim(0, 30)
ax.broken_barh([(lo, timedelta(hours=1))], (10, 20))

plt.show()
```

**Actual outcome**

```
Traceback (most recent call last):
  File ".\problem.py", line 11, in <module>
    ax.broken_barh([(lo, timedelta(hours=1))], (10, 20))
  File "C:\...\lib\site-packages\matplotlib\__init__.py", line 1810, in inner
    return func(ax, *args, **kwargs)
  File "C:\...\lib\site-packages\matplotlib\axes\_axes.py", line 2528, in broken_barh
    xranges = self.convert_xunits(xranges)
  File "C:\...\lib\site-packages\matplotlib\artist.py", line 186, in convert_xunits
    return ax.xaxis.convert_units(x)
  File "C:\...\lib\site-packages\matplotlib\axis.py", line 1530, in convert_units
    ret = self.converter.convert(x, self.units, self)
  File "C:\...\lib\site-packages\matplotlib\dates.py", line 1801, in convert
    return date2num(value)
  File "C:\...\lib\site-packages\matplotlib\dates.py", line 428, in date2num
    return _to_ordinalf_np_vectorized(d)
  File "C:\...\lib\site-packages\numpy\lib\function_base.py", line 1972, in __call__
    return self._vectorize_call(func=func, args=vargs)
  File "C:\...\lib\site-packages\numpy\lib\function_base.py", line 2048, in _vectorize_call
    outputs = ufunc(*inputs)
  File "C:\...\lib\site-packages\matplotlib\dates.py", line 229, in _to_ordinalf
    base = float(dt.toordinal())
AttributeError: 'datetime.timedelta' object has no attribute 'toordinal'
```

**Expected outcome**

A graph showing a single bar from 20:00 to 21:00.

If this is a limitation of the current version of matplotlib, I'd be perfectly happy with a workaround that gave the expected result by another means (but the approaches I've tried have resulted in the axis tick marks not being correctly formatted as times).

**Matplotlib version**
  * Operating system: Windows 7
  * Matplotlib version: 3.0.2
  * Matplotlib backend (`print(matplotlib.get_backend())`): TkAgg
  * Python version: 3.7.1
  * Jupyter version (if applicable): N/A
  * Other libraries: None

Python installed from the python.org Windows 64-bit installer. Matplotlib installed from the wheel on PyPI (`pipenv install matplotlib`).

Hints from issue discussion:
Can you provide the full example with the `timedelta64` array that produces that error? I've encountered similar looking problems before, that depend on when we do our unit conversions, and this might actually be fixed in the master branch.
@dstansby - the full example is:

```python
import numpy as np
import matplotlib.pyplot as plt
x = np.array(range(100), dtype='M8[D]')
ax = plt.subplot(1,1,1)
y = np.random.random(100)
ax.bar(x[:-1], y[:-1], width=np.diff(x))
```

The problem is our unit conversion. We do
~~~
x = self.convert_xunits(x)
width = self.convert_xunits(width)
~~~

However, we don't have a conversion implemented for `timedelta64`. A result, `x` is our internal float-based date format while `width` is still a `timedelta64`, which cannot be subtracted from each other.

The solution would be to implement a converter for `timedelta64`.
Ah, so this would be fixed by something like https://github.com/matplotlib/matplotlib/pull/9120 I think.
Sort of. It needs a bit more infrastructure:

- implement a converter analogously to `DateConverter`
- add it to `matplotlib.units.registry`
I'm having a look at this now, but having trouble with your example @astrofrog ; when I do
```python
import numpy as np
x = np.array(range(100), dtype='M8[D]')
print((x[:-1] + np.diff(x) / 2.0) - x[:-1])
```
I just get a bunch of zeros; dividing `np.diff(x)` by a number seems to just zero it out. Do you know of a way to fix this? Internally `Matplotlib` has to divide the width by 2, which is why I can't get your example working.
`'M8[D]'` has a single day resolution, and your array goes up by one, so dividing by two rounds down to 0. You'd need to at least change it to hourly resolution.
@dstansby, not sure of the approach you are taking here, but I think as @timhoffm says, we should add timedelta64 to dates.py, and register it.  Then all the math can be done in floating point numbers...
Trying to take that approach, but running into the problem that `np.timedelta64` is a subclass of `numbers.Number`, so the conversion interface assumes it doesn't need converting:

```python
import numpy as np
import matplotlib.pyplot as plt
import matplotlib.units as munits
x = np.array(range(100), dtype='M8[D]')
xdiff = np.diff(x)
print(xdiff.dtype)
print(munits.ConversionInterface.is_numlike(xdiff))
```
prints
```python
timedelta64[D]
True
```

#12863 should fix this now, after some pain.  Its a bit difficult for there to be essentially two "units" in a single list, but its a perfectly valid use case, so...

Commit message: FIX: brokenbarh math before units

TST: Add test of broken_barh
FIX: make bar work with timedeltas
TST: check bar and barh work with timedelta
DOC: fix private method docstring
FIX: revert list comprehension

TST: fix test
FIX: throw error xr not tw-tuple

Patch to review:
diff --git a/lib/matplotlib/axes/_axes.py b/lib/matplotlib/axes/_axes.py
index fd724f55876c..473df00d2759 100644
--- a/lib/matplotlib/axes/_axes.py
+++ b/lib/matplotlib/axes/_axes.py
@@ -2011,6 +2011,26 @@ def step(self, x, y, *args, where='pre', data=None, **kwargs):
         kwargs['drawstyle'] = 'steps-' + where
         return self.plot(x, y, *args, data=data, **kwargs)
 
+    @staticmethod
+    def _convert_dx(dx, x0, x, convert):
+        """
+        Small helper to do logic of width conversion flexibly.
+
+        *dx* and *x0* have units, but *x* has already been converted
+        to unitless.  This allows the *dx* to have units that are
+        different from *x0*, but are still accepted by the  ``__add__``
+        operator of *x0*.
+        """
+        try:
+            # attempt to add the width to x0; this works for
+            # datetime+timedelta, for instance
+            dx = convert(x0 + dx) - x
+        except (TypeError, AttributeError):
+            # but doesn't work for 'string' + float, so just
+            # see if the converter works on the float.
+            dx = convert(dx)
+        return dx
+
     @_preprocess_data()
     @docstring.dedent_interpd
     def bar(self, x, height, width=0.8, bottom=None, *, align="center",
@@ -2172,23 +2192,25 @@ def bar(self, x, height, width=0.8, bottom=None, *, align="center",
         else:
             raise ValueError('invalid orientation: %s' % orientation)
 
+        x, height, width, y, linewidth = np.broadcast_arrays(
+            # Make args iterable too.
+            np.atleast_1d(x), height, width, y, linewidth)
+
         # lets do some conversions now since some types cannot be
         # subtracted uniformly
         if self.xaxis is not None:
+            x0 = x
             x = self.convert_xunits(x)
-            width = self.convert_xunits(width)
+            width = self._convert_dx(width, x0, x, self.convert_xunits)
             if xerr is not None:
-                xerr = self.convert_xunits(xerr)
+                xerr = self._convert_dx(xerr, x0, x, self.convert_xunits)
 
         if self.yaxis is not None:
+            y0 = y
             y = self.convert_yunits(y)
-            height = self.convert_yunits(height)
+            height = self._convert_dx(height, y0, y, self.convert_yunits)
             if yerr is not None:
-                yerr = self.convert_yunits(yerr)
-
-        x, height, width, y, linewidth = np.broadcast_arrays(
-            # Make args iterable too.
-            np.atleast_1d(x), height, width, y, linewidth)
+                yerr = self._convert_dx(yerr, y0, y, self.convert_yunits)
 
         # Now that units have been converted, set the tick locations.
         if orientation == 'vertical':
@@ -2465,10 +2487,19 @@ def broken_barh(self, xranges, yrange, **kwargs):
         self._process_unit_info(xdata=xdata,
                                 ydata=ydata,
                                 kwargs=kwargs)
-        xranges = self.convert_xunits(xranges)
-        yrange = self.convert_yunits(yrange)
-
-        col = mcoll.BrokenBarHCollection(xranges, yrange, **kwargs)
+        xranges_conv = []
+        for xr in xranges:
+            if len(xr) != 2:
+                raise ValueError('each range in xrange must be a sequence '
+                                 'with two elements (i.e. an Nx2 array)')
+            # convert the absolute values, not the x and dx...
+            x_conv = self.convert_xunits(xr[0])
+            x1 = self._convert_dx(xr[1], xr[0], x_conv, self.convert_xunits)
+            xranges_conv.append((x_conv, x1))
+
+        yrange_conv = self.convert_yunits(yrange)
+
+        col = mcoll.BrokenBarHCollection(xranges_conv, yrange_conv, **kwargs)
         self.add_collection(col, autolim=True)
         self.autoscale_view()
 
diff --git a/lib/matplotlib/tests/test_axes.py b/lib/matplotlib/tests/test_axes.py
index 6e75ce726963..c8e2a00e0b15 100644
--- a/lib/matplotlib/tests/test_axes.py
+++ b/lib/matplotlib/tests/test_axes.py
@@ -1498,6 +1498,32 @@ def test_barh_tick_label():
             align='center')
 
 
+def test_bar_timedelta():
+    """smoketest that bar can handle width and height in delta units"""
+    fig, ax = plt.subplots()
+    ax.bar(datetime.datetime(2018, 1, 1), 1.,
+           width=datetime.timedelta(hours=3))
+    ax.bar(datetime.datetime(2018, 1, 1), 1.,
+           xerr=datetime.timedelta(hours=2),
+           width=datetime.timedelta(hours=3))
+    fig, ax = plt.subplots()
+    ax.barh(datetime.datetime(2018, 1, 1), 1,
+            height=datetime.timedelta(hours=3))
+    ax.barh(datetime.datetime(2018, 1, 1), 1,
+            height=datetime.timedelta(hours=3),
+            yerr=datetime.timedelta(hours=2))
+    fig, ax = plt.subplots()
+    ax.barh([datetime.datetime(2018, 1, 1), datetime.datetime(2018, 1, 1)],
+            np.array([1, 1.5]),
+            height=datetime.timedelta(hours=3))
+    ax.barh([datetime.datetime(2018, 1, 1), datetime.datetime(2018, 1, 1)],
+            np.array([1, 1.5]),
+            height=[datetime.timedelta(hours=t) for t in [1, 2]])
+    ax.broken_barh([(datetime.datetime(2018, 1, 1),
+                     datetime.timedelta(hours=1))],
+                   (10, 20))
+
+
 @image_comparison(baseline_images=['hist_log'],
                   remove_text=True)
 def test_hist_log():
@@ -5433,6 +5459,15 @@ def test_broken_barh_empty():
     ax.broken_barh([], (.1, .5))
 
 
+def test_broken_barh_timedelta():
+    """Check that timedelta works as x, dx pair for this method """
+    fig, ax = plt.subplots()
+    pp = ax.broken_barh([(datetime.datetime(2018, 11, 9, 0, 0, 0),
+                          datetime.timedelta(hours=1))], [1, 2])
+    assert pp.get_paths()[0].vertices[0, 0] == 737007.0
+    assert pp.get_paths()[0].vertices[2, 0] == 737007.0 + 1 / 24
+
+
 def test_pandas_pcolormesh(pd):
     time = pd.date_range('2000-01-01', periods=10)
     depth = np.arange(20)


Code review comments:
diff_hunk: @@ -2172,23 +2192,25 @@ def bar(self, x, height, width=0.8, bottom=None, *, align="center",
         else:
             raise ValueError('invalid orientation: %s' % orientation)
 
+        x, height, width, y, linewidth = np.broadcast_arrays(
path: lib/matplotlib/axes/_axes.py
text: @user1:
Should this go below unit conversion?  Otherwise I think units will be dropped if width is a list of unitized stuff?

@author:
```python
y = [1 ,2, 4]
w = [timedelta(hours=2), timedelta(hours=3), timedelta(hours=3)]

print(np.broadcast_arrays(y, w))
```
I don't think this drops units.  

```
[array([1, 2, 4]), array([datetime.timedelta(seconds=7200),
       datetime.timedelta(seconds=10800),
       datetime.timedelta(seconds=10800)], dtype=object)]
```

@user1:
This will drop stuff like pint units or astropy units.
I remember some discussion a while ago which basically reached the conclusion that we cannot call *any* numpy function before unit handling (tbh I was rather unhappy about that, but the goal is not to relitigate this here), perhaps @user2 will remember this better.

@author:
Can someone provide an example?   This works fine for all the categorical, unit, and date tests.  If we are supporting something that doesn’t survive broadcasting we need to start testing it.  

I’m not sure why we broadcast here at all anyway.  

@user3:
AFAICS, broadcasting is used to support scalar parameters such as `plt.bar([1, 2, 3], [3,5,2], width=0.5)`

@user2:
For the record, `broadcast_arrays` **will** drop pint units--it might possibly work with astropy since it's an ndarray subclass. Broadcasting itself works fine, it's calling functions that need to allocate new arrays that's the problem; there are currently no hooks to allow overriding what type to use when allocating an empty array.

@author:
Agreed - sorry, should have responded here - I did move the broadcast after the convert...