BUG: fix for #4753 (out keyword with __numpy_ufunc__) - #5566

I haven't looked at the code yet, but your summary note has me scared and
confused! What is this about breaking scoping? Can you elaborate?
On 12 Feb 2015 23:23, "Blake Griffith" [email protected] wrote:

This address bug #4753 #4753

It should be noted that while we now check the out keyword arg for an
overriding object, the third party numpy_ufunc method needs to
implement its own breaking of function scoping to actually assign the
result to whatever is in kwds['out'] outside the function.

So when we do

c = OverrideObject()
some_ufunc(a, b, out=c)

if we actually want c to contain the result like:

c = some_ufunc(a, b)

Then in OverrideObject.numpy_ufunc you would have to access c (which
is out of scope) and set it as the result.

To fix this I would probably have to integrate the override stuff more

into ufunc argument processing. I'm not sure how to do this yet.

You can view, comment on, or merge this pull request online at:

#5566
Commit Summary

• Check out kwarg for nump_ufunc override
• Tests for bug 4753

File Changes

• M numpy/core/src/private/ufunc_override.h
  https://github.com/numpy/numpy/pull/5566/files#diff-0 (82)
• M numpy/core/tests/test_multiarray.py
  https://github.com/numpy/numpy/pull/5566/files#diff-1 (31)

Patch Links:

• https://github.com/numpy/numpy/pull/5566.patch
• https://github.com/numpy/numpy/pull/5566.diff

—
Reply to this email directly or view it on GitHub
#5566.

I don't understand the meaning of this comment either. ufuncs do not break scoping, whether they are overridden or not?

If I understand the meaning correctly, in the statement "... if we actually want c to contain the result like ..." the answer is that we certainly don't want this --- the non-overridden ufuncs do not do this sort of magic either.

@pv @njsmith I think I was confusing myself with how numpy handles the out kwarg... This actually is fine.

Basically I as confused about how this would work if kwarg['out'] was immutable, in which case it can't really be passed by reference. So mutating the value of kwarg['out'] would be impossible (without accessing the namespace outside of the function "breaking function scoping") but the kwarg['out'] thing should always be mutable. So it doesn't matter.

Yeah, if kwarg["out"] were immutable then the correct thing to do would be
to try to mutate it anyway and then propagate the resulting error. Even
breaking scoping wouldn't let us mutate the kwarg["out"] object if it were
immutable -- all it would let us do is mutate a binding in the outer
function's scope, so that it pointed to a different object entirely.
On Feb 16, 2015 2:22 AM, "Blake Griffith" [email protected] wrote:

Basically I as confused about how this would work if kwarg['out'] was
immutable, in which case it can't really be passed by reference. So
mutating the value of kwarg['out'] would be impossible (without accessing
the namespace outside of the function "breaking function scoping") but the
kwarg['out'] thing should always be mutable. So it doesn't matter.

—
Reply to this email directly or view it on GitHub
#5566 (comment).

I updated the tests when I realized what was going on. This is ready for review.

Where is this at now?

Still waiting for review. ping @pv @njsmith

ping @pv @njsmith

@cowlicks - just to add that I think from the perspective of a developer using this, having to deal with out in the __numpy_ufunc__ is definitely what I would expect to have to do. Indeed, quite a bit of Quantity.__numpy_ufunc__ does exactly that: https://github.com/mhvk/astropy/blob/quantity-numpy-ufunc-additions/astropy/units/quantity.py#L414

I will rebase and take a look at #5674 tomorrow and see what needs to change here. I think my original incorrect comments on this PR are just confusing everyone. So I'll make a new PR when I'm done and close this.

Thanks @mhvk since that seems to be the general expectation.