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

staticmethod causes wrong arguments-differ #7556

Closed
kayhayen opened this issue Oct 3, 2022 · 4 comments
Closed

staticmethod causes wrong arguments-differ #7556

kayhayen opened this issue Oct 3, 2022 · 4 comments
Labels
Duplicate 🐫 Duplicate of an already existing issue

Comments

@kayhayen
Copy link

kayhayen commented Oct 3, 2022

Bug description

The argument parsing mishandles uses of staticmethod in base classes for methods and miscounts the arguments, considering self extra.

Code is

class C:
    @staticmethod
    def meth():
        pass
class D(C):
    def meth(self):
        pass

Since staticmethods are faster to call, I have this frequently mixed and only use non-staticmethods where absolutely necessary. Base class implementations are often pessimistic and return None, False, do nothing etc.

Configuration

No configuration

Command used

pylint m.py

Pylint output

m.py:6 W0221 arguments-differ D.meth Number of parameters was 0 in 'C.meth' and is now 1 in overridden 'D.meth' method

Expected behavior

Should not give that warning.

Pylint version

pylint 2.15.3
astroid 2.12.10
Python 3.9.2 (default, Feb 28 2021, 17:03:44)

OS / Environment

Debian Buster with PyPI packages, likely irrelevant

Additional dependencies

No response

@kayhayen kayhayen added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Oct 3, 2022
@jacobtylerwalls
Copy link
Member

arguments-differ is very picky about self, which is part of the reason for using it. I hear that you wish it wouldn't count self, but at this point that's part of the identity of the message.

IIUC the example you present here violates LSP, as D can't be substituted for C's contract:

>>> C.meth()
>>> D.meth()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: D.meth() missing 1 required positional argument: 'self'

More discussion at #6865 (for the reverse scenario that does not violate LSP). It sounds like you wish to disable arguments-differ in your config.

@jacobtylerwalls jacobtylerwalls closed this as not planned Won't fix, can't repro, duplicate, stale Oct 4, 2022
@jacobtylerwalls jacobtylerwalls added Duplicate 🐫 Duplicate of an already existing issue and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Oct 4, 2022
@kayhayen
Copy link
Author

kayhayen commented Oct 7, 2022

There are valid uses for arguments-differ in my mind, so disabling it globally is likely to miss out on useful reports, which would be a pity. The LSP violation topic, I must admit, I get it, but I feel like it deserves its own warning. I did not figure out, what LSP stands for. I would appreciate a link if you have any.

In my mind it's PyLint that warns about methods that do not use self, and suggests to the user to make them staticmethod, kind of implying it's not a real change, and I always agreed with that, but now I learn that was wrong all along.

Local disables cannot be used to annotate the one in the base class, or can they?

class C:
    @staticmethod
    def meth(): # pylint: disable=arguments-differ
        pass
class D(C):
    def meth(self):
        pass

@jacobtylerwalls
Copy link
Member

Liskov Substitution Principle

@jacobtylerwalls
Copy link
Member

Local disables cannot be used to annotate the one in the base class, or can they?

No, but I believe you could disable the message for part of the module, right before the base class, and then enable it again after all of your subclasses. (If the idea is to not have disables on each subclass.) See: https://pylint.pycqa.org/en/latest/user_guide/messages/message_control.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate 🐫 Duplicate of an already existing issue
Projects
None yet
Development

No branches or pull requests

2 participants