BUG/DOC/MAINT: Tidy up histogramdd #10802
Conversation
This also switches to doing comparisons rather than subtractions, for consistency with np.histogram. That change is not strictly necessary here as the arguments are not unsigned integer types (unlike in np.histogram), but it would nice to support integer bins in future.
…ension Previously gave `ValueError: object too deep for desired array` from an internal call This also adds support for 0d array bincounts
cd72495 to
4600b40
Compare
| The data to be histogrammed. It must be an (N,D) array or data | ||
| that can be converted to such. The rows of the resulting array | ||
| are the coordinates of points in a D dimensional polytope. | ||
| sample : (N, D) array, or (D, N) array_like |
numpy/lib/histograms.py
Outdated
| sample : (N, D) array, or (D, N) array_like | ||
| The data to be histogrammed. | ||
|
|
||
| If an array, each row is a coordinate in a D-dimensional space. |
There was a problem hiding this comment.
This might look better as a list rather than separate paragraphs. Why the transpose for array_like?
There was a problem hiding this comment.
Transpose was always there, not my choice - just documented it.
It's so the caller can pass (X, Y, Z) instead of np.stack((X, Y, Z), axis=-1), I think, which is similar to some matplotlib APIs.
I don't like it, but I think we're stuck with it.
numpy/lib/histograms.py
Outdated
| @@ -792,8 +795,10 @@ def histogramdd(sample, bins=10, range=None, normed=False, weights=None): | |||
|
|
|||
| range : sequence, optional | |||
| A sequence of lower and upper bin edges to be used if the edges are | |||
| not given explicitly in `bins`. Defaults to the minimum and maximum | |||
| values along each dimension. | |||
| not given explicitly in `bins`. An entry of None in the sequence | |||
There was a problem hiding this comment.
So they must match the dimensions in some way?
There was a problem hiding this comment.
Yep, should be a tuple of length D. Will update.
|
Seems like a somewhat weird function. The documentation still isn't very clear. |
|
Do you want me to leave the DOC commit to a later PR? |
|
If you still want to work on the documentation, it would be good to at least get the fix in. |
…gramdd This also adds support for inferring the range along a subset of the axes, rather than an all or nothing approach.
4600b40 to
c8a5f56
Compare
|
@charris: docstring updated, and a release note added for the new feature |
|
Much nicer. Thanks Eric. |
Split into small commits, one piece at a time.
Fixes gh-8531.
Happy to split these commits into multiple PRs, if needed.