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

False positive for cell-var-from-loop / W0640 #7100

Open
tushar-deepsource opened this issue Jun 30, 2022 · 4 comments
Open

False positive for cell-var-from-loop / W0640 #7100

tushar-deepsource opened this issue Jun 30, 2022 · 4 comments
Labels
Documentation 📗 False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@tushar-deepsource
Copy link
Contributor

tushar-deepsource commented Jun 30, 2022

Sounds crazy, but I think W0640 (cell-var-from-loop) is a lint for an issue that is non-existent.

Here's the code snippet from the example, which should supposedly cause i to be assigned to the last value in the list:

def foo(numbers):
    for i in numbers:
        def bar():
            print(i)
        bar()

Except, it works fine:

>>> foo([1, 2, 3])
1
2
3

Same is true for lambdas:

def foo(numbers):
    for num in numbers:
        print(list(filter(lambda x: x != num, numbers)))
>>> foo([1, 2, 3])
[2, 3]
[1, 3]
[1, 2]

Here's pylint output:

asd.py:3:41: W0640: Cell variable num defined in loop (cell-var-from-loop)

Pylint version

pylint 2.13.7
astroid 2.11.3
Python 3.10.4 (main, Apr 26 2022, 19:36:29) [Clang 13.1.6 (clang-1316.0.21.2)]
@tushar-deepsource tushar-deepsource added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jun 30, 2022
@Pierre-Sassoulas
Copy link
Member

I think our example is wrong in the documentation because we wanted to make it small but in the process we made it meaningless. It works fine because you're using the function directly in the loop and you could simply do:

def foo(numbers):
    for i in numbers:
        print(i)

But https://stackoverflow.com/questions/25314547/cell-var-from-loop-warning-from-pylint explains why the check is useful. I'd welcome a less naive example for the documentation :)

@Pierre-Sassoulas Pierre-Sassoulas added Documentation 📗 Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jun 30, 2022
@tushar-deepsource
Copy link
Contributor Author

Documentation aside, it's still a false positive for this simple case:

def foo(numbers):
    for num in numbers:
        print(list(filter(lambda x: x != num, numbers)))

Do you think it's safe to not raise this issue for these cases?

@Pierre-Sassoulas Pierre-Sassoulas added the False Positive 🦟 A message is emitted but nothing is wrong with the code label Jun 30, 2022
@tushar-deepsource
Copy link
Contributor Author

tushar-deepsource commented Jul 2, 2022

I think I get it now.

def get_functions(numbers):
    for num in numbers:
        function = lambda: print(num)
        yield function

for func in get_functions():
    func()

This one works:

1
2
3

This one also works:

def get_functions(numbers):
    for num in numbers:
        function = lambda: print(num)
        yield function

functions = get_functions([1, 2, 3])
for func in functions:
    func()
1
2
3

BUT, this one doesn't work:

def get_functions(numbers):
    for num in numbers:
        function = lambda: print(num)
        yield function

functions = list(get_functions([1, 2, 3]))
for func in functions:
    func()
3
3
3

It's because list(...) eagerly collects the functions, but for-loops lazily yield the functions. If you eagerly evaluate the functions, the value of num gets to 3 before the function is ever called.

@Pierre-Sassoulas
Copy link
Member

#5263 is a near duplicate.

@Pierre-Sassoulas Pierre-Sassoulas changed the title W0640 is wrong False positive for cell-var-from-loop / W0640 Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation 📗 False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

2 participants