You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Bincount with integer weights should return integer array #6854
It is more of a feature than a bug... When present, the weights array is always casted to double before binning. This makes the C code simple, and provides a less surprising experience in case of overflow.
What exactly is your use case that needs the return to be of integer type? Of what integer type exactly? How would you expect overflow to be handled, if at all?
As the user I expect it not to handle overflows at all (like many other operations - e.g. adding two arrays ignores overflows) and return the same type as weights are. Actually I thought it is this way already, but noticed that I can't add the result of bincount to my accumulator array (of int32's) - so I use np.bincount(...).astype(np.int32). Not a big deal of course, but it seems surprising. Another point is that it uses 2x extra memory for int32 or float32 weights, converting them to float64.
So, if it would greatly complicate the C code, may be it's not worth changing. But as I understand, at least with Cython it shouldn't be any problem at all. Not sure if Cython is widely used in numpy, however.
I'm +1 for making bincount work "as expected" regarding the dtype of the weights.
In my usecase, I need np.bincount([0,0,1], weights=[1j,2j,1j]) to return [3j, 1j], whereas now it raises a TypeError because it tries to convert the complex weights to float. Unfortunately, the hack/fix for this case involves calling bincount twice, handling the real and imaginary sides separately.
I think it makes sense from an API standpoint to have np.bincount's dtype rules behave the same as np.sum in the presence of weights. After all, they're conceptually doing the same thing.
I do understand the pain of writing native code that handles lots of dtypes, but I think the benefit outweighs the cost here.
I have just submitted a PR (#7464) addressing this, and have sent a message to the mailing list about the changes. It would be good if you could weigh in the discussion, as I assume there are going to be a number of controversial points to solve before this can be merged.
It appears that #7464 has been abandoned. Matching the type of weights seems reasonable to me and I would like to see this feature. @jaimefrio Do you think your PR could be re-opened? Could I help with it?
Activity
jaimefrio commentedon Dec 18, 2015
It is more of a feature than a bug... When present, the weights array is always casted to
doublebefore binning. This makes the C code simple, and provides a less surprising experience in case of overflow.What exactly is your use case that needs the return to be of integer type? Of what integer type exactly? How would you expect overflow to be handled, if at all?
aplavin commentedon Dec 18, 2015
As the user I expect it not to handle overflows at all (like many other operations - e.g. adding two arrays ignores overflows) and return the same type as weights are. Actually I thought it is this way already, but noticed that I can't add the result of
bincountto my accumulator array (ofint32's) - so I usenp.bincount(...).astype(np.int32). Not a big deal of course, but it seems surprising. Another point is that it uses 2x extra memory forint32orfloat32weights, converting them tofloat64.So, if it would greatly complicate the C code, may be it's not worth changing. But as I understand, at least with Cython it shouldn't be any problem at all. Not sure if Cython is widely used in numpy, however.
rgommers commentedon Jan 3, 2016
No. It could be used more widely, but currently there's only Cython code in
numpy.random.rgommers commentedon Jan 3, 2016
@jaimefrio close this as wontfix then?
perimosocordiae commentedon Mar 5, 2016
I'm +1 for making bincount work "as expected" regarding the dtype of the weights.
In my usecase, I need
np.bincount([0,0,1], weights=[1j,2j,1j])to return[3j, 1j], whereas now it raises a TypeError because it tries to convert the complex weights to float. Unfortunately, the hack/fix for this case involves calling bincount twice, handling the real and imaginary sides separately.I think it makes sense from an API standpoint to have
np.bincount's dtype rules behave the same asnp.sumin the presence of weights. After all, they're conceptually doing the same thing.I do understand the pain of writing native code that handles lots of dtypes, but I think the benefit outweighs the cost here.
jaimefrio commentedon Mar 26, 2016
I have just submitted a PR (#7464) addressing this, and have sent a message to the mailing list about the changes. It would be good if you could weigh in the discussion, as I assume there are going to be a number of controversial points to solve before this can be merged.
ENH: Make bincount output same type as weights, fixes numpy#6854
bdice commentedon May 19, 2020
It appears that #7464 has been abandoned. Matching the type of
weightsseems reasonable to me and I would like to see this feature. @jaimefrio Do you think your PR could be re-opened? Could I help with it?rossbar commentedon May 19, 2020
Here's a link to relevant discussion on the mailing list: https://mail.python.org/pipermail/numpy-discussion/2016-March/ (search for "bincount" to get the relevant thread, about halfway down the page).
The consensus at the time seemed to be for type-promotion behavior consistent with sum FWIW.