Skip to content
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

tensorflow: Tensor.__bool__'s signature is incorrect for one element tensors #13365

Closed
hoel-bagard opened this issue Jan 5, 2025 · 2 comments · Fixed by #13562
Closed

tensorflow: Tensor.__bool__'s signature is incorrect for one element tensors #13365

hoel-bagard opened this issue Jan 5, 2025 · 2 comments · Fixed by #13562

Comments

@hoel-bagard
Copy link
Contributor

CurrentlyTensor.__bool__'s signature is as follows:

class Tensor:
    def __bool__(self) -> NoReturn: ...

It suggests that the method should never be called, however the following is valid code:

import tensorflow as tf

bool(tf.constant([1]))

Since this depends on the shape of the tensor, I don't know if this can be fixed, but it would be nice to have.

@srittau
Copy link
Collaborator

srittau commented Feb 27, 2025

Cc @hmc-cs-mdrissi who originally contributed the tensorflow stubs.

@hoel-bagard hoel-bagard changed the title tensorfllow: Tensor.__bool__'s signature is incorrect for one element tensors tensorflow: Tensor.__bool__'s signature is incorrect for one element tensors Feb 28, 2025
@hmc-cs-mdrissi
Copy link
Contributor

There's two reasons for this.

  1. It only works for scalars and fails with other shapes.
  2. It only works in some execution modes. It'll be fine in eager, but can crash for graph mode even with scalars. Most code I work with needs to be graph mode compatible.

I'm fine with the change. My experience leans that those two issues cover large percent of usages, but it's fair that policy is prefer false negatives and I don't have actual numbers for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants