MAINT: Remove duplicated logic between array_wrap and array_prepare #10459
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mhvk
approved these changes
Jan 23, 2018
| @@ -243,27 +269,13 @@ _find_array_prepare(PyObject *args, PyObject *kwds, | |||
| obj = PyDict_GetItem(kwds, npy_um_str_out); | |||
There was a problem hiding this comment.
You are on purpose not yet checking whether the out argument is a tuple, correct?
There was a problem hiding this comment.
I'm on purpose just leaving it as it was - I want to push the out argument parsing way upstream
|
Actually, I should try to keep the comments from the function I removed - so don't merge this yet |
eric-wieser
force-pushed
the
func_lookup_deduplicate
branch
from
326ac85 to
ee0a016
Compare
|
Updated with comments and slightly more efficient code. |
mhvk
reviewed
Jan 24, 2018
numpy/core/src/umath/ufunc_object.c
Outdated
| static PyObject * | ||
| _get_output_array_method(PyObject *obj, PyObject *method, | ||
| PyObject *input_method) { | ||
| if (obj == Py_None) { |
There was a problem hiding this comment.
There are now three times in which you have the incref and return of input_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
eric-wieser
force-pushed
the
func_lookup_deduplicate
branch
from
ee0a016 to
e6956dd
Compare
|
Should be ready |
|
All OK now, merging! |
eric-wieser
added a commit
to eric-wieser/numpy
that referenced
this pull request
Jan 26, 2018
…lt 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 because numpygh-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.
charris
pushed a commit
to charris/numpy
that referenced
this pull request
Feb 9, 2018
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 because numpygh-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.
hanjohn
pushed a commit
to hanjohn/numpy
that referenced
this pull request
Feb 15, 2018
…lt 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 because numpygh-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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This behaves exactly as before
Part of the cleanup needed to fix #10450