Skip to content

ENH: Adds an out argument to bincount. - #9424

Closed
jaimefrio wants to merge 1 commit into
numpy:mainfrom
jaimefrio:bincount_out
Closed

ENH: Adds an out argument to bincount.#9424
jaimefrio wants to merge 1 commit into
numpy:mainfrom
jaimefrio:bincount_out

Conversation

@jaimefrio

@jaimefrio jaimefrio commented Jul 14, 2017

Copy link
Copy Markdown
Member

Fixes #9397.

@jaimefrio

jaimefrio commented Jul 14, 2017

Copy link
Copy Markdown
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
Comment thread 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);

@eric-wieser eric-wieser Jul 14, 2017

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.

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

@jaimefrio jaimefrio Jul 15, 2017

Copy link
Copy Markdown
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.

@seberg seberg Jul 15, 2017

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.

I expect the biggest problem was what do about backcompat.

@mhvk mhvk Dec 22, 2018

Copy link
Copy Markdown
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

charris commented Jul 15, 2017

Copy link
Copy Markdown
Member

Test failure looks legitimate.

@charris

charris commented Jul 18, 2017

Copy link
Copy Markdown
Member

Couple of compiler warnings to deal with.

@mhvk

mhvk commented Dec 22, 2018

Copy link
Copy Markdown
Contributor

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

GregoryMorse commented Dec 2, 2019

Copy link
Copy Markdown

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

mattip commented Aug 10, 2022

Copy link
Copy Markdown
Member

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