Repository navigation
Conversation
By mistake, any arguments to ufunc.reduce, ufunc.accumulate, and ufunc.reduceat that were None were removed, rather than just removing the 'out' argument. This is corrected here, with tests added.
mhvk
added
09 - Backport-Candidate
eric-wieser
left a comment
eric-wieser
left a comment
There was a problem hiding this comment.
Needs a test with out != None, because that will fail!
| if (i == 3) { | ||
| Py_DECREF(obj); | ||
| if (i == 3) { | ||
| /* remove out=None */ |
There was a problem hiding this comment.
Does this remove kwarg['out'] == None too?
There was a problem hiding this comment.
No, that is removed in PyUFunc_CheckOverride before we even get here.
| } | ||
| obj = PyTuple_GetSlice(args, 3, 4); | ||
| } | ||
| PyDict_SetItemString(*normal_kwds, kwlist[i], obj); |
There was a problem hiding this comment.
This isn't going to fly - out must be a single value for reduce, not a tuple
There was a problem hiding this comment.
Darn, you're absolutely right. As is, things simply break with out given. I think fixing it here is better.
There was a problem hiding this comment.
At which point, I'd argue that for reduce and accumulate, never passing out=None is just unhelpful - can we do the opposite, and always set it?
|
Although perhaps fixing |
|
Can you rebase on 14ff219 to make the backport easy? |
|
@eric-wieser - see #9105 - I think it has now become clear we should discuss what to do with |
|
Is this PR superseded by #9106? |
|
Yes, if you think #9106 is OK, then this can be dropped. |
|
Closing in favor of #9106. |
3 participants
By mistake, any arguments to ufunc.reduce, ufunc.accumulate, and ufunc.reduceat that were None were removed, rather than just removing the 'out' argument. This is corrected here, with tests added.
fixes #9102