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

Replace != None with is not None in Python bindings #375

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

stephane-caron
Copy link
Contributor

@stephane-caron stephane-caron commented Feb 11, 2025

In Python it is better to compare to not None (typo fixed, thanks @nim65s) with is or is not rather than == and != (e.g. the comparison operators may not be defined).

In this code it changes nothing, because tensors are from PyTorch:

In [1]: import torch

In [2]: x = torch.tensor([1, 2, 3])

In [3]: x is not None
Out[3]: True

In [4]: x != None
Out[4]: True

But with NumPy arrays it would yield, for instance:

In [1]: x = np.array([1, 2, 3])

In [2]: x is not None
Out[2]: True

In [3]: x != None
Out[3]: array([ True,  True,  True])

In [4]: if x != None:
   ...:     pass
   ...: 
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[4], line 1
----> 1 if x != None:
      2     pass

ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

@hrp2-14
Copy link
Member

hrp2-14 commented Feb 11, 2025

Hi ! This project doesn't usually accept pull requests on the main branch.
If this wasn't intentionnal, you can change the base branch of this PR to devel
(No need to close it for that). Best, a bot.

@stephane-caron stephane-caron changed the base branch from main to devel February 11, 2025 16:34
Copy link
Member

@nim65s nim65s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have been found by some linter :)

NB: typo: not -> None

@nim65s nim65s mentioned this pull request Feb 11, 2025
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 this pull request may close these issues.

4 participants