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

Do not use len(SEQUENCE) to determine if a sequence is empty #2559

Closed

Conversation

DimitriPapadopoulos
Copy link
Collaborator

From DeepSource.io:
Built-in function len used as condition PYL-C1802

Using the len function to check if a sequence is empty is not idiomatic and can be less performant than checking the truthiness of the object.

len doesn't know the context in which it is called, so if computing the length means traversing the entire sequence, it must; it doesn't know that the result is just being compared to 0. Computing the boolean value can stop after it sees the first element, regardless of how long the sequence actually is.

Addresses #2552 (comment).

From DeepSource.io:
https://deepsource.io/directory/analyzers/rust/issues/PYL-C1802

Using the len function to check if a sequence is empty is not idiomatic
and can be less performant than checking the truthiness of the object.

len doesn't know the context in which it is called, so if computing the
length means traversing the entire sequence, it must; it doesn't know
that the result is just being compared to 0. Computing the boolean value
can stop after it sees the first element, regardless of how long the
sequence actually is.
@cclauss
Copy link
Contributor

cclauss commented Oct 25, 2022

https://pypi.org/project/refurb (and probably other tools) can auto-discover these as FURB115

Some do not like this change because the current form makes it explicit that the object is a container and not merely a bool.

@larsoner
Copy link
Member

In this case we know more than it just being a generic sequence, we know it's a list. So to me there's no real gain here. I think I'm with @cclauss's statement in the sense that we do actually lose a bit of information here, so -0.5 from me

@DimitriPapadopoulos
Copy link
Collaborator Author

No problem, in that case do not hesitate to close this PR.

@larsoner
Copy link
Member

Thanks for the helpful discussions, but let's keep it explicit here!

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.

3 participants