ENH: Allow ufunc.identity to be any python object by eric-wieser · Pull Request #8955 · numpy/numpy

eric-wieser · 2017-04-17T22:47:08Z

~~The first commit in this PR is #8952, which probably ought to be merged first~~ (done)

Fixes #7702 and #4599, and also a related issue to #8860

eric-wieser · 2017-04-17T22:47:41Z

Previously we never did any error checking here, which seems wrong

eric-wieser · 2017-04-17T22:49:55Z

Zero wasn't really correct, but there was previously no way to specify False

homu · 2017-05-01T10:19:46Z

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

charris · 2017-10-02T16:33:32Z

Needs rebase. There seem to be several PRs in this series that should go in together.

eric-wieser · 2018-02-02T09:17:03Z

Updated

hameerabbasi · 2018-02-19T10:55:19Z

I would suggest adding identities to np.minimum and np.maximum, which would be +inf and -inf respectively. This is intuitively and logically true, as max(a, -inf) = a and min(a, +inf) = a.

xref #5032

eric-wieser · 2018-02-20T08:10:46Z

@hameerabbasi: I think that's a bad idea (think integer types, max(abs(x)), ...), but let's have that discussion at the linked issue, not here.

eric-wieser · 2018-02-20T08:13:11Z

(base changed to make github recompute the diff)

mhvk · 2018-03-21T12:39:41Z

From https://github.com/numpy/numpy/pull/10635/files#r170466944 - once #10635 is merged, we should be sure to correct the code such that the initializer is also used as one of the elements over which reductions are done for object dtype (in #10635, it is only used for empty slices).

eric-wieser · 2018-11-05T16:52:43Z

This might make sense to get into 1.16, while we're adding members to ufuncs

mhvk · 2018-11-05T17:02:43Z

Indeed, I can review again if rebased.

eric-wieser · 2018-11-06T06:29:39Z

Rebased, with some more docs

eric-wieser · 2018-11-06T07:50:59Z

I'm not really experienced enough with C apis to know whether this is a good idea - does CPython provide any guidelines on when to steal and not to steal references?

Generally, it seems to be stolen only when the reference can reasonably expected to be stored elsewhere (as in setting an element of a list). That does seem to be the case here. That said, the case is less clear than for a list, and the documentation [1] explicitly states that reference stealing functions are exceptions. So, my sense would be not to steal the reference.

[1] https://docs.python.org/3/c-api/intro.html#reference-count-details

mhvk

The code looks good (modulo the discussion about reference stealing, where I think we perhaps better not).

But there should be a test case...

mhvk · 2018-11-06T14:40:32Z

Generally, it seems to be stolen only when the reference can reasonably expected to be stored elsewhere (as in setting an element of a list). That does seem to be the case here. That said, the case is less clear than for a list, and the documentation [1] explicitly states that reference stealing functions are exceptions. So, my sense would be not to steal the reference.

[1] https://docs.python.org/3/c-api/intro.html#reference-count-details

eric-wieser · 2018-11-10T20:26:04Z

@mhvk: Updated with a slightly longer testcase

eric-wieser · 2018-11-10T20:29:01Z

Note: This will need a rebase if #11977 goes in first, even though github won't show conflicts.

mhvk

Looks all OK to me

mhvk · 2018-11-10T21:28:40Z

Shall we merge now to avoid the need for a rebase? I think this is all OK.

eric-wieser · 2018-11-10T22:11:59Z

Yeah, why not. Updated with release notes.

eric-wieser · 2018-11-11T04:25:17Z

@mattip has said he'll hold off on #11977 until this goes in

mattip · 2018-11-12T16:28:44Z

Shouldn't this be 1.16 API?

Fixed, good catch

Fixes numpy#7702

mattip · 2018-11-12T17:55:46Z

Thanks Eric

The lack of identity for `logaddexp2` was first identitifed in numpy#4599. The implementation in numpy#8955 added -inf as identity for `logaddexp`, but missed adding it for `logaddexp2`.

The lack of identity for `logaddexp2` was first identitifed in #4599. The implementation in #8955 added -inf as identity for `logaddexp`, but missed adding it for `logaddexp2`. Co-Authored-By: Eric Wieser <wieser.eric@gmail.com> Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>

eric-wieser added 01 - Enhancement component: numpy._core labels Apr 17, 2017

eric-wieser commented Apr 17, 2017

View reviewed changes

eric-wieser force-pushed the obj-identity branch 2 times, most recently from 52afba2 to 97e36ad Compare April 18, 2017 09:12

This was referenced Apr 18, 2017

ENH: add identity kwarg to frompyfunc #8255

Merged

MAINT: Removed duplicated code around ufunc->identity #8952

Merged

eric-wieser force-pushed the obj-identity branch 2 times, most recently from 907dc3e to 8cea62b Compare May 1, 2017 10:47

eric-wieser mentioned this pull request May 4, 2017

Missing identity for logaddexp #4599

Closed

eric-wieser commented Jul 8, 2017

View reviewed changes

Comment thread numpy/core/include/numpy/ufuncobject.h Outdated

eric-wieser mentioned this pull request Oct 25, 2017

min/max of empty arrays #5032

Closed

eric-wieser force-pushed the obj-identity branch from 8cea62b to e180357 Compare February 2, 2018 09:16

This was referenced Feb 2, 2018

MAINT: Use new-style format strings for clarity #10509

Merged

MAINT: Allow errors to escape from InitOperators #10516

Merged

eric-wieser force-pushed the obj-identity branch from e180357 to 27d9f1a Compare February 3, 2018 19:16

eric-wieser changed the base branch from master to maintenance/1.14.x February 20, 2018 08:11

eric-wieser changed the base branch from maintenance/1.14.x to master February 20, 2018 08:12

numpy deleted a comment from hameerabbasi Feb 20, 2018

eric-wieser mentioned this pull request Feb 21, 2018

Sum/product of empty object array is False/True #10639

Closed

hameerabbasi mentioned this pull request Feb 23, 2018

ENH: Implement initial kwarg for ufunc.add.reduce #10635

Merged

eric-wieser force-pushed the obj-identity branch from 27d9f1a to aa50061 Compare November 6, 2018 06:27

eric-wieser force-pushed the obj-identity branch from aa50061 to e475e76 Compare November 6, 2018 07:04

eric-wieser commented Nov 6, 2018

View reviewed changes

mhvk reviewed Nov 6, 2018

View reviewed changes

eric-wieser force-pushed the obj-identity branch 2 times, most recently from 08204dd to 77347a3 Compare November 10, 2018 20:25

eric-wieser commented Nov 10, 2018

View reviewed changes

Comment thread numpy/core/tests/test_umath.py Outdated

eric-wieser Nov 10, 2018

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.

@mhvk: Updated with a slightly longer testcase

eric-wieser mentioned this pull request Nov 10, 2018

BUG: reference cycle in np.vectorize #11977

Merged

mhvk approved these changes Nov 10, 2018

View reviewed changes

mattip reviewed Nov 12, 2018

View reviewed changes

eric-wieser added 3 commits November 12, 2018 09:14

ENH: Allow ufunc.identity to be any python object

d3b2036

ENH: Correct identities for logical ufuncs and logaddexp

aedc758

Fixes numpy#7702

DOC: Add release notes on changes to ufunc.identity

e044ae3

eric-wieser force-pushed the obj-identity branch from 42d1fe2 to e044ae3 Compare November 12, 2018 17:16

mattip approved these changes Nov 12, 2018

View reviewed changes

mattip merged commit e34f5bb into numpy:master Nov 12, 2018

eric-wieser mentioned this pull request Nov 19, 2018

MismatchCAPIWarning during build #12412

Closed

eriknw mentioned this pull request Apr 28, 2020

ENH: correct identity for logaddexp2 ufunc: -inf #16102

Merged

Uh oh!

Conversation

eric-wieser commented Apr 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

homu commented May 1, 2017

Uh oh!

Uh oh!

charris commented Oct 2, 2017

Uh oh!

eric-wieser commented Feb 2, 2018

Uh oh!

hameerabbasi commented Feb 19, 2018 • edited by eric-wieser Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eric-wieser commented Feb 20, 2018

Uh oh!

eric-wieser commented Feb 20, 2018

Uh oh!

mhvk commented Mar 21, 2018

Uh oh!

eric-wieser commented Nov 5, 2018

Uh oh!

mhvk commented Nov 5, 2018

Uh oh!

eric-wieser commented Nov 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-wieser commented Nov 10, 2018

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

mhvk commented Nov 10, 2018

Uh oh!

eric-wieser commented Nov 10, 2018

Uh oh!

eric-wieser commented Nov 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattip commented Nov 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eric-wieser commented Apr 17, 2017 •

edited

Loading

hameerabbasi commented Feb 19, 2018 •

edited by eric-wieser

Loading

eric-wieser commented Nov 6, 2018 •

edited

Loading