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 comparison-with-callable when comparing two callables #7724

Open
sam-s opened this issue Nov 7, 2022 · 8 comments
Open

False positive comparison-with-callable when comparing two callables #7724

sam-s opened this issue Nov 7, 2022 · 8 comments
Labels
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

@sam-s
Copy link

sam-s commented Nov 7, 2022

Bug description

W0143(comparison-with-callable) is issued for no good reason, when both parts are callable:

"""test
"""
def f0(x):
    "identity"
    return x
def f1(x):
    "add 1"
    return x+1

def main(f, x):
    "test function"
    if f == f0:
        print("identity")
    else:
        print(f.__name__)
    return f(x)

here the clear intent is to compare callables, so the warning appears to be unnecessary.
While I can disable it, it seems that it should not be issued when both parts of the comparison are callable.

Configuration

none

Command used

pylint a.py

Pylint output

a.py:12: [W0143(comparison-with-callable), main] Comparing against a callable, did you omit the parenthesis?

Expected behavior

no diagnostics

Pylint version

pylint 2.15.5
astroid 2.12.12
Python 3.10.6 (main, Nov  2 2022, 18:53:38) [GCC 11.3.0]

OS / Environment

popos 22.04

Additional dependencies

No response

@sam-s sam-s added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Nov 7, 2022
@Pierre-Sassoulas Pierre-Sassoulas added 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 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Nov 7, 2022
@Pierre-Sassoulas Pierre-Sassoulas changed the title Too aggressive comparison-with-callable False positive comparison-with-callable when comparing two callables Nov 7, 2022
@DanielNoord
Copy link
Collaborator

There is no way for pylint to infer that f is a callable, even if you were to add type information. So, although I do think we should not raise comparison-with-callable for callable comparisons we would need a piece of code that pylint would actually be able to understand.

Sorry for not being able to help, but I'm closing this issue until somebody has a reproducer that we would be reasonably able to infer.

@Pierre-Sassoulas Pierre-Sassoulas closed this as not planned Won't fix, can't repro, duplicate, stale Nov 7, 2022
@sam-s
Copy link
Author

sam-s commented Nov 7, 2022

But f is called in this function!
I thought it was enough to infer that it is callable.

At any rate, what do you suggest I do to avoid the warning?
Annotate?
Or replace f == f0 with f.__name__ == f0.__name__ ?

Thank you.

@Pierre-Sassoulas
Copy link
Member

We don't have the definition of f and we'd need #4813 to be able to take the type annotation into account. I think you should disable the message locally, maybe on the whole project if you use this design pattern a lot (I would not recommend changing the code only to appease pylint about a false positive).

@sam-s
Copy link
Author

sam-s commented Nov 7, 2022

How about restricting the warning to the case when you can infer that one side in the comparison is callable while the other is not?

@Pierre-Sassoulas Pierre-Sassoulas added Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning and removed Won't fix/not planned labels Nov 7, 2022
@Pierre-Sassoulas
Copy link
Member

Sure we usually favor false negatives when the inference fail, we should also do that here.

@Pierre-Sassoulas Pierre-Sassoulas removed the Needs investigation 🔬 A bug or crash where it's not immediately obvious what is happenning label Nov 7, 2022
@tushar-deepsource
Copy link
Contributor

Why not use f is f1? That should suppress the issue as well.

@michaelgrewal
Copy link

michaelgrewal commented Sep 29, 2023

is doesn't work for me when comparing callables within a Class.

class Test():
    def __init__(self):
        self.name="test"
    
    def foo(self):
        print("foo")
        
    def run(self):
        print(self.bar(self.foo))
        
    def bar(self, method):
        return method is self.foo  # False

t = Test()
t.run()

If I use == to compare functions then it works but I get the pylint error. I also don't like to disable pylint warnings with comments.

If I use method.__func__ is self.foo.__func__ to compare it works but doesn't look clean.

I'm using python 3.10

@tusharsadhwani
Copy link
Contributor

Comparing methods with == is too niche of a use case, and as you said you can work around that (by passing Test.foo instead of self.foo for example will be comparable with is)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

6 participants