Conversation
numpy/core/src/umath/ufunc_object.c
Outdated
There was a problem hiding this comment.
Almost all of this function, and its doc-comment, are a direct move of the code from before, with the exception that:
goto failis justreturn -1- some fields need re-extracting from ufunc
ufunc_nameis recalculated when needed - only in errors, so performance is irrelevant
numpy/core/src/umath/ufunc_object.c
Outdated
There was a problem hiding this comment.
Starting at 0 makes more sense, because our function works by assigning to it until its not zero
There was a problem hiding this comment.
It's always overwritten, so the value assigned here is irrelevant, right?
There was a problem hiding this comment.
True, but this the correct base case if we end up with a code path that never sets retval, so seems like a good thing to keep
jaimefrio
left a comment
There was a problem hiding this comment.
Even after this it's still almost a 500 LOC function, way too much, so this is a step in the right direction, I would say.
numpy/core/src/umath/ufunc_object.c
Outdated
There was a problem hiding this comment.
You don't need this assignment anymore, do you?
There was a problem hiding this comment.
I don't follow - why not?
There was a problem hiding this comment.
Oh, wait, yes I do...
numpy/core/src/umath/ufunc_object.c
Outdated
There was a problem hiding this comment.
It's always overwritten, so the value assigned here is irrelevant, right?
|
I see the point, though #8819 will mean that it will need do_remap and remap_axis arguments, which means it will be less stand-alone than it is now. |
|
I don't think that needing to add extra parameters in future is an argument against this function - even if it needs more inputs, it still has one goal, which is to produce the argument passed to the inner loop. Also, when passed |
Previously the ufunc methods would use "(unknown)", but the basic __call__ would use "<unknown ufunc>"
98bbfa8 to
19ae9fb
Compare
|
@jaimefrio: Fixed that stray |
|
Not at all -- I think that one hasn't converged, and I can rebase if need be. My main point was that, with it, the case for splitting this part off became a bit less strong, since now more of the function's state has to be passed on. |
This was more about making it clear which state these loops are producing, that about making it clear what state they take as input |
|
I'd like to pull the trigger on this before we accumulate more technical debt in the ufunc machinery |
|
I'm OK with this being merged; and certainly we should merge the name part. |
|
In it goes then! |
Follows on from numpygh-8876
ufunc->name == NULL