Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork11.9k
MAINT: Remove duplicated logic between array_wrap and array_prepare#10459
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
mhvk left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Only a question about whether you'd not rather check for a tuple here as well. Fine to do that in a follow-up PR, though.
numpy/core/src/umath/ufunc_object.c Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You are on purpose not yet checking whether theout argument is a tuple, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm on purpose just leaving it as it was - I want to push theout argument parsing way upstream
eric-wieser commentedJan 23, 2018
Actually, I should try to keep the comments from the function I removed - so don't merge this yet |
326ac85 toee0a016Compareeric-wieser commentedJan 24, 2018
Updated with comments and slightly more efficient code. |
numpy/core/src/umath/ufunc_object.c Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There are now three times in which you have the incref and return ofinput_method. How about inverting the logic, and move the next two lines to the very end, as the fall-back opion, i.e.,
PyObject *ometh;if obj != Py_None` { if (PyArrayCheckExact(obj)) { return Py_RETURN_NONE; } ometh = ... if (ometh) { if(PyCallable_Check(ometh)) { return ometh; } else { Py_DECREF(ometh); } } else { PyErr_Clear(); }}Py_XINCREF(input_method);return input_method;This behaves exactly as before
ee0a016 toe6956ddCompareeric-wieser commentedJan 24, 2018
Should be ready |
mhvk commentedJan 25, 2018
All OK now, merging! |
…lt to inherit the output's maskThis brings `np.add(a, b, out)` in line with `np.add(a, b, out=out)`.These previously differed becausenumpygh-10459 causes them to call __array_wrap__ in different ways (with and without the output argument in the context tuple, respectively).Since the data in the `out` argument is never used by ufuncs, it seems consistent that the mask should not be either.
Currently, this causes the result to inherit the output's mask.This brings `np.add(a, b, out)` in line with `np.add(a, b, out=out)`.These previously differed becausenumpygh-10459 causes them to call__array_wrap__ in different ways (with and without the output argumentin the context tuple, respectively).Since the data in the `out` argument is never used by ufuncs, it seemsconsistent that the mask should not be either.
…lt to inherit the output's maskThis brings `np.add(a, b, out)` in line with `np.add(a, b, out=out)`.These previously differed becausenumpygh-10459 causes them to call __array_wrap__ in different ways (with and without the output argument in the context tuple, respectively).Since the data in the `out` argument is never used by ufuncs, it seems consistent that the mask should not be either.
Uh oh!
There was an error while loading.Please reload this page.
This behaves exactly as before
Part of the cleanup needed to fix#10450