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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MAINT: deduplicate check_nonreorderable_axes #10309
MAINT: deduplicate check_nonreorderable_axes #10309
Conversation
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.
Looks good with one small comment.
numpy/core/src/umath/reduction.c
Outdated
@@ -496,30 +487,29 @@ PyUFunc_ReduceWrapper(PyArrayObject *operand, PyArrayObject *out, | |||
goto fail; | |||
} | |||
|
|||
/* |
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 think this can be moved one if-statement further up, so that it goes together with the sanity check on wheremask
(and then one can just return NULL
instead of goto fail
).
Actually, it might even make more sense in ufunc_object
, before we even get here (right after getting the identity, which reorderable depends on), so that one doesn't have to pass reorderable
in at all, but that is perhaps too much of a change, and may be better left for when/if we unify reduction and single-axis ufuncs.
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.
Good points
e6f4a2a
to
1a09222
Compare
Updated with a little more cleanup |
1a09222
to
4693e68
Compare
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.
Nice further cleanup; good to have the error raised in the main routine.
Call was present in both branches of the
if
statementFound while looking at #834