BUG/DEP: Make ufunclike functions more ufunc-like #8996
Conversation
eric-wieser
force-pushed
the
fix-ufunclike
branch
2 times, most recently
from
6618d12
to
3a1e139
Compare
eric-wieser
changed the title
numpy/lib/tests/test_ufunclike.py
Outdated
| assert_warns(DeprecationWarning, ufl.fix, [1, 2], y=nx.empty(2)) | ||
| assert_warns(DeprecationWarning, ufl.isposinf, [1, 2], y=nx.empty(2)) | ||
| assert_warns(DeprecationWarning, ufl.isneginf, [1, 2], y=nx.empty(2)) | ||
| def test_scalar(self): |
There was a problem hiding this comment.
@need a blank line between functions.
eric-wieser
force-pushed
the
fix-ufunclike
branch
from
3a1e139
to
6c06883
Compare
| y = nx.empty(x.shape, dtype=nx.bool_) | ||
| nx.logical_and(nx.isinf(x), nx.signbit(x), y) | ||
| return y | ||
| return nx.logical_and(nx.isinf(x), nx.signbit(x), out) |
There was a problem hiding this comment.
For a little more efficiency, we could change this to:
nx.logical_and(nx.isinf(x, out), nx.signbit(x), out)
But that seemed out of scope for the bugfix/deprecation
eric-wieser
force-pushed
the
fix-ufunclike
branch
from
6c06883
to
e4f2726
Compare
|
Ugh, this was much messier than intended - turns out that #8994 is a much bigger stumbling block to making things work right than expected. |
eric-wieser
force-pushed
the
fix-ufunclike
branch
from
e4f2726
to
20b49ad
Compare
eric-wieser
force-pushed
the
fix-ufunclike
branch
from
20b49ad
to
d2f35b6
Compare
| warnings.warn( | ||
| "The name of the out argument to {} has changed from `y` to " | ||
| "`out`, to match other ufuncs.".format(f.__name__), | ||
| DeprecationWarning, stacklevel=3) |
There was a problem hiding this comment.
Previous commit forgot to specify a stacklevel
|
(Since there's some subclass trickery going on here, and you've had opinions on |
numpy/lib/ufunclike.py
Outdated
| return y | ||
|
|
||
| def isposinf(x, y=None): | ||
| return _where(nx.greater_equal(x, 0), nx.floor(x), nx.ceil(x), out) |
There was a problem hiding this comment.
The version of where you have above is not subclass-safe, at least not for Quantity (where, in general, the two arrays might have different units). For this particular function, it might nevertheless work, but I would suggest not introducing a new function. Instead, perhaps the following (I checked it passes all tests):
out = nx.asanyarray(nx.floor(x, out=out))
out = nx.ceil(x, out=out, where=(x < 0))
return out if out.shape else out[()]
There was a problem hiding this comment.
This should be out if type(out) != np.ndarray or out.shape else out[()], I think.
There was a problem hiding this comment.
It should be whatever is closest to what a ufunc does; any subclasses should be able to deal with out[()]... (I guess the alternative would be to separate out the non-array case, but that seems overkill)
There was a problem hiding this comment.
Yep, ufuncs only unpack scalars on the base classes, presumably as subclasses might lose metadata
There was a problem hiding this comment.
Alright, fixed I think - it was a little bit messier than that
There was a problem hiding this comment.
And one more time, with tests for the cases that your suggestion didn't cover
eric-wieser
force-pushed
the
fix-ufunclike
branch
from
d2f35b6
to
6e2424b
Compare
No need to reinvent the wheel here - the ufunc machinery will handle the out arguments Fixes numpy#8993
eric-wieser
force-pushed
the
fix-ufunclike
branch
from
6e2424b
to
05f4228
Compare
|
Looks all OK now! |
|
Could probably add the |
|
I'm happy with this. @charris - do you want to have another look, or is this ready to go in? |
|
@mhvk I didn't really review, just glanced over it and thought it was generally good. Put it in if you like it. |
|
OK, merged! Thanks, @eric-wieser! |
This fixes #8993 and #8995