Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ENH: Adds an out argument to bincount. #9424

Closed
wants to merge 1 commit into from
Closed

ENH: Adds an out argument to bincount. #9424

wants to merge 1 commit into from

Conversation

jaimefrio
Copy link
Member

Fixes #9397.

@jaimefrio
Copy link
Member Author

It's missing all the necessary extra tests and additional documentation, and it uses out=, which doesn't seem to be a crowd favorite. But the implementation is all done and seems to be working fine on some informal testing.

@jaimefrio jaimefrio mentioned this pull request Jul 14, 2017
Open
eric-wieser
eric-wieser reviewed Jul 14, 2017
View reviewed changes
numpy/core/src/multiarray/compiled_base.c
}

lst = (PyArrayObject *)PyArray_ContiguousFromAny(list, NPY_INTP, 1, 1);
if (lst == NULL) {
list_arr = _array_from_object(list_obj, NPY_INTP, 0);
Copy link
Member

Choose a reason for hiding this comment

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

Why is the output type intp? That seems somewhat arbitrary, and it would seem more sensible to respect the type of the out argument

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm... Perhaps we could mix in here some of the ideas in #7464, which I apparently abandoned for no good reason. Will give it some thought.

Copy link
Member

Choose a reason for hiding this comment

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

I expect the biggest problem was what do about backcompat.

Copy link
Contributor

Choose a reason for hiding this comment

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

For this PR, isn't it fine to keep the list integer?

@charris
Copy link
Member

charris commented Jul 15, 2017

Test failure looks legitimate.

@charris
Copy link
Member

charris commented Jul 18, 2017

Couple of compiler warnings to deal with.

@mhvk
Copy link
Contributor

mhvk commented Dec 22, 2018

This is still a nice idea. I'm not quite sure what got it stuck...

To get around the weirdness of not initializing out, one could also introduce an initial argument (and, then, if initial is out, one has no initialization).

@GregoryMorse
Copy link

Please fix this PR as it would be a useful addition.

Base automatically changed from master to main March 4, 2021 02:04
@InessaPawson InessaPawson added the 52 - Inactive Pending author response label Jun 8, 2022
@mattip
Copy link
Member

mattip commented Aug 10, 2022

Closing since it has not been touched for quite a while. This may be useful, if someone else wants to pick it up.

@mattip mattip closed this Aug 10, 2022
@mhvk mhvk mentioned this pull request Oct 23, 2022
Open
@arthurfeeney arthurfeeney mentioned this pull request Dec 31, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seberg seberg seberg left review comments

@eric-wieser eric-wieser eric-wieser left review comments

@mhvk mhvk mhvk left review comments

Assignees
No one assigned
Labels
01 - Enhancement 52 - Inactive Pending author response component: numpy._core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature request: Incremental bincount and axis argument
8 participants
@jaimefrio @charris @mhvk @GregoryMorse @mattip @seberg @eric-wieser @InessaPawson