Skip to content

Conversation

@seberg
Copy link
Member

@seberg seberg commented Aug 10, 2021

While this makes sense, the implementation here does not actually
work. It is very difficult to actually reach it, but if reached
it just crashes the process, so remove it.

(I am not actually sure why the input needs to be object already
here, that seems to be an issue with the current dispatcher/promotion
special casing object a bit oddly.)


Stumbled/struggled with it for the ufunc reductions refactor, so splitting it out, since it is a bit strange inside that PR.

EDIT: The tests are a bit obscure, so here is a simpler repro:

np.logical_and([True], [False, None], signature="OO->?")

While this makes sense, the implementation here does not actually
work.  It is very difficult to actually reach it, but if reached
it just crashes the process, so remove it.

(I am not actually sure why the input needs to be object already
here, that seems to be an issue with the current dispatcher/promotion
special casing object a bit oddly.)
@seberg seberg added 00 - Bug 09 - Backport-Candidate PRs tagged should be backported labels Aug 10, 2021
@seberg
Copy link
Member Author

seberg commented Aug 10, 2021

Marking as backport candidate, since I think it should be OK to backport.

@mattip
Copy link
Member

mattip commented Aug 11, 2021

So this should not have been added in #14800 ?

@seberg
Copy link
Member Author

seberg commented Aug 11, 2021

Yeah, should have been wrong from the start, we just misjudged the out mechanism to actually do more than set the signature I guess. Plus actually picking that loop seems harder than it should be, so it is almost impossible to trigger.

@mattip mattip merged commit cbe8b45 into numpy:main Aug 11, 2021
@mattip
Copy link
Member

mattip commented Aug 11, 2021

Thanks @seberg

@seberg seberg deleted the logical-obj-ufuncs-and-bool branch August 11, 2021 14:17
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants