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 arguments-differ when converting @staticmethod to/from bound method #6019

Closed
tl24 opened this issue Mar 28, 2022 · 3 comments
Closed
Labels
C: arguments-differ Issues related to 'arguments-differ' and 'signature-differs' checks Discussion 🤔

Comments

@tl24
Copy link

tl24 commented Mar 28, 2022

Bug description

When changing a base class static method with the @staticmethod decorator to a bound method using self, the arguments-differ warning is being generated. I don't believe this should be the case as calling the child class method using self works correctly.

# pylint: disable=too-few-public-methods
class BaseClass:
    @staticmethod
    def handlers():
        return {}


class ChildClass(BaseClass):
    def handlers(self):
        return {"foo": self.foo_handler}

    def foo_handler(self):
        pass

Command used

pylint tests/args_differ.py

Pylint output

************* Module tests.args_differ
tests/args_differ.py:9:4: W0221: Number of parameters was 0 in 'BaseClass.handlers' and is now 1 in overridden 'ChildClass.handlers' method (arguments-differ)

Expected behavior

No warnings generated

Pylint version

pylint 2.13.2
astroid 2.11.2
Python 3.9.9 (main, Nov 15 2021, 18:05:17) 
[GCC 5.4.0 20160609]

OS / Environment

Ubuntu 20.04

@tl24 tl24 added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Mar 28, 2022
@Pierre-Sassoulas Pierre-Sassoulas added False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Mar 28, 2022
@Pierre-Sassoulas Pierre-Sassoulas changed the title false positive arguments-differ when converting @staticmethod to bound method False positive arguments-differ when converting @staticmethod to/from bound method Mar 28, 2022
@cdce8p cdce8p added Discussion 🤔 C: arguments-differ Issues related to 'arguments-differ' and 'signature-differs' checks and removed False Positive 🦟 A message is emitted but nothing is wrong with the code labels Mar 28, 2022
@cdce8p
Copy link
Member

cdce8p commented Mar 28, 2022

I don't believe this should be the case as calling the child class method using self works correctly.

True, but the other way doesn't work. You can't replace a call to Base.handlers() with Child.handlers(). If you know that you'll be calling the method with self.xxx anyway, it probably shouldn't be a static one.

Btw. both pyright and mypy also emits an error here.

# pyright
Method "handlers" overrides class "BaseClass" in an incompatible manner
  Positional parameter count mismatch; base method has 0, but override has 1

# mypy
error: Signature of "handlers" incompatible with supertype "BaseClass"  [override]
note:      Superclass:
note:          @staticmethod
note:          def handlers() -> Dict[Any, Any]
note:      Subclass:
note:          def handlers(self) -> Dict[Any, Any]

@tl24
Copy link
Author

tl24 commented Mar 29, 2022

Ok, so is the guidance here just to disable the "no-self-use" warning in the base class if the intent is for it to be overridden in the child class and possibly needing the self reference?

@cdce8p
Copy link
Member

cdce8p commented Mar 29, 2022

Ok, so is the guidance here just to disable the "no-self-use" warning in the base class if the intent is for it to be overridden in the child class and possibly needing the self reference?

Exactly! IMO the no-self-use checker encourages the wrong behavior here. Just because the base implementation of a method doesn't use self, doesn't mean it should be a static method. I opened an issue for that a while ago

@cdce8p cdce8p closed this as completed Mar 29, 2022
Cito added a commit to WebwareForPython/w4py3 that referenced this issue Jul 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: arguments-differ Issues related to 'arguments-differ' and 'signature-differs' checks Discussion 🤔
Projects
None yet
Development

No branches or pull requests

3 participants