Skip to content

BUG: Fix handling of mixed-type arrays in histogram#7864

Closed
zkillpack wants to merge 1 commit intonumpy:masterfrom
zkillpack:histogram-isfinite-fix
Closed

BUG: Fix handling of mixed-type arrays in histogram#7864
zkillpack wants to merge 1 commit intonumpy:masterfrom
zkillpack:histogram-isfinite-fix

Conversation

@zkillpack
Copy link
Copy Markdown

@zkillpack zkillpack commented Jul 22, 2016

Arrays that contain both floats and single-element float arrays behave inconsistently in np.histogram:

Running this works as expected:

>>> np.histogram(np.asarray([np.array([0.4]) for i in range(10)] + [-np.inf]))
ValueError: range parameter must be finite.

Running this does not:

>>> np.histogram(np.asarray([np.array([0.4]) for i in range(10)] + [np.inf]))
TypeError: ufunc 'isfinite' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''

Similarly, this gives the expected result:

>>> np.histogram(np.asarray([np.array([0.5]) for i in range(10)] + [.50000000000000001]))
(array([ 0,  0,  0,  0,  0, 11,  0,  0,  0,  0]),
 array([ 0. ,  0.1,  0.2,  0.3,  0.4,  0.5,  0.6,  0.7,  0.8,  0.9,  1. ]))

Running this does not:

>>> np.histogram(np.asarray([np.array([0.5]) for i in range(10)] + [.5000000000000001]))
TypeError: ufunc 'isfinite' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''

Since arrays and floats both implement max() and min(), the bounds-checking code at the beginning of histogram() will happily take the max and min, and then inconsistently infer the dtype depending on the size of the array's contents, e.g.:

>>> np.asarray([4, np.array([np.inf])]).dtype`
dtype('float64')

>>> np.asarray([np.array([-np.inf]), 4]).dtype`
dtype('O')

@njsmith
Copy link
Copy Markdown
Member

njsmith commented Jul 22, 2016

Sorry, I'm having trouble figuring out what behavior you expect here. I would have expected that passing histogram an array containing another array would always be an error. Is that what you expect to?

@zkillpack
Copy link
Copy Markdown
Author

I decided to allow this rather than raise an error, given that single-element arrays implement enough interfaces for this to work, were it not for the bug. In any case, the current behavior is inconsistent: if the input is degenerate, it shouldn't allow it in some cases, and give an unexpected error in others.

@homu
Copy link
Copy Markdown
Contributor

homu commented Feb 21, 2017

☔ The latest upstream changes (presumably #8584) made this pull request unmergeable. Please resolve the merge conflicts.

@eric-wieser
Copy link
Copy Markdown
Member

To me, it feels like np.asarray([4, np.array([np.inf])]) has incorrect behaviour right now, and should not be flattening the nested array

raise ValueError(
'max must be larger than min in range parameter.')
if not np.all(np.isfinite([mn, mx])):
if not np.all(np.isfinite(np.hstack([mn, mx]))):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can of worms would be better solved with np.isfinite(mn) and np.isinfinite(mx)

@charris charris closed this in 99f605c Dec 24, 2017
charris referenced this pull request Dec 24, 2017
BUG: Fix misleading error when coercing to array
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants