Repository: numpy/numpy
Pull request: MAINT: fix histogram*d dispatchers
Fixes GH-13728

CC @mhvk

<!-- Please be sure you are following the instructions in the dev guidelines
http://www.numpy.org/devdocs/dev/development_workflow.html
-->

<!-- We'd appreciate it if your commit message is properly formatted
http://www.numpy.org/devdocs/dev/development_workflow.html#writing-the-commit-message
-->


Problem statement:
histogradd dispatcher does not dispatch correctly on bins
As a reminder for myself if nobody else gets around to it: `bins` in `histogramdd` is a sequence of arrays, not an array itself, so one should dispatch on things inside it (if iterable).

EDIT: `samples` may also be interpreted wrong, since it can be a sequence of arrays.

Hints from issue discussion:
And `histogram2d` also does not dispatch correctly on `bins`
Should we document this ?
@abhinavsagar - we should fix it! Ideally before 1.17 comes out...
I'll cc @shoyer just in case he has time before me.

Commit message: MAINT: fix histogram*d dispatchers

fixes GH-13728

Patch to review:
diff --git a/numpy/lib/histograms.py b/numpy/lib/histograms.py
index 5bcedc7f44cd..478fbc783e57 100644
--- a/numpy/lib/histograms.py
+++ b/numpy/lib/histograms.py
@@ -3,6 +3,7 @@
 """
 from __future__ import division, absolute_import, print_function
 
+import contextlib
 import functools
 import operator
 import warnings
@@ -922,7 +923,13 @@ def histogram(a, bins=10, range=None, normed=None, weights=None,
 
 def _histogramdd_dispatcher(sample, bins=None, range=None, normed=None,
                             weights=None, density=None):
-    return (sample, bins, weights)
+    if hasattr(sample, 'shape'):  # same condition as used in histogramdd
+        yield sample
+    else:
+        yield from sample
+    with contextlib.supress(TypeError):
+        yield from bins
+    yield weights
 
 
 @array_function_dispatch(_histogramdd_dispatcher)
diff --git a/numpy/lib/twodim_base.py b/numpy/lib/twodim_base.py
index e165c9b02dfb..1928aa10c4c7 100644
--- a/numpy/lib/twodim_base.py
+++ b/numpy/lib/twodim_base.py
@@ -565,7 +565,13 @@ def vander(x, N=None, increasing=False):
 
 def _histogram2d_dispatcher(x, y, bins=None, range=None, normed=None,
                             weights=None, density=None):
-    return (x, y, bins, weights)
+    yield x
+    yield y
+    if hasattr(bins, 'shape'):  # same condition as used in histogramdd
+        yield bins
+    else:
+        yield from bins
+    return weights
 
 
 @array_function_dispatch(_histogram2d_dispatcher)


Code review comments:
diff_hunk: @@ -922,7 +923,13 @@ def histogram(a, bins=10, range=None, normed=None, weights=None,
 
 def _histogramdd_dispatcher(sample, bins=None, range=None, normed=None,
                             weights=None, density=None):
-    return (sample, bins, weights)
+    if hasattr(sample, 'shape'):  # same condition as used in histogramdd
+        yield sample
+    else:
+        yield from sample
+    with contextlib.supress(TypeError):
path: numpy/lib/histograms.py
text: @user1:
Suppress has two `p`s.

I'd probably `try / except` on just the length here though:
```
try:
    len(bins)
except TypeError:
    pass
else:
    yield from bins
```

@user2:
Actually prefer the `contextlib` one for its brevity. And compared to the time taken to histogram anything it is probably not too expensive.

@user1:
The difference is that this silences exceptions within `bins.__iter__().__next__()`, which you could argue it should not