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

R0903 should consider nontrivial __init__ #8976

Closed
dgutson opened this issue Aug 25, 2023 · 6 comments
Closed

R0903 should consider nontrivial __init__ #8976

dgutson opened this issue Aug 25, 2023 · 6 comments

Comments

@dgutson
Copy link

dgutson commented Aug 25, 2023

Bug description

I think that R0903 should not be triggered when there is a public method AND an __init__ method containing code/logic, since it is indeed another public method.

for example:

def f() -> int:
    return 1

class C:
    def __init__(self, x: int) -> None:
        self.x = x + 1
        self.y = x + f()

    def sum(self) -> int:
        return self.x + self.y

Configuration

No response

Command used

pylint r0903.py

Pylint output

r0903.py:4:0: R0903: Too few public methods (1/2) (too-few-public-methods)

Expected behavior

__init__ should be considered a public method too.

Pylint version

pylint 2.15.3
astroid 2.12.10
Python 3.10.12

OS / Environment

No response

Additional dependencies

No response

@dgutson dgutson added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Aug 25, 2023
@DanielNoord
Copy link
Collaborator

Dundee methods aren't considered public. It would be very hard to determine when there is "sufficient" logic to consider __init__ nontrivial.

Sorry but I don't think we will implement this.

@DanielNoord DanielNoord closed this as not planned Won't fix, can't repro, duplicate, stale Aug 26, 2023
@DanielNoord DanielNoord added Won't fix/not planned and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Aug 26, 2023
@dgutson
Copy link
Author

dgutson commented Aug 26, 2023

  1. What if I implement this? Or someone working with me provides a PR?
  2. There are ways to determine the level of logic of a method, ranging from e.g. looking for control structures to cyclomatic complexity

I think closing this issue just now without a bit of discussion is a bit premature.

@dgutson
Copy link
Author

dgutson commented Aug 26, 2023

Another example of metric: Depth of nested loops and conditionals: Functions with deeper nested loops and conditionals are likely to have more complex logic

@Pierre-Sassoulas
Copy link
Member

The issue is that we need to maintain it later. So we'd accept this PR and then we have someone else that want to actually want to consider non-trivial __str__ and __add__, another one don't want to consider __init__ over 3 lines and under 5 lines as non trivial and another persons want us to use Mc Cabe cyclomatic complexity and not the number of line, and yet another one just want the old behavior back. So we'd end up adding 3 options, discuss it for hours, and review a dozen PRs (+/- 5 for the bugs the new behavior creates). It's better if this check is kept simple and stupid and is enabled on a case by case basis (enabled because we're going to disable it by default in #3512 / pylint 3.0). I would suggest to create an external pylint extension if you want something different / more refined.

@mbyrnepr2
Copy link
Member

mbyrnepr2 commented Aug 26, 2023

Dundee methods aren't considered public. It would be very hard to determine when there is "sufficient" logic to consider __init__ nontrivial.

Sorry but I don't think we will implement this.

At least in terms of the current Pylint logic, I think that isn't correct. See this comment and the following lines.

__init__ is the only dunder currently excluded from what is considered a public method. That seems unexpected to me - why exclude __init__ and not __str__, __repr__ etc.?

class PrivateMethodsOnly:
    def __str__(self):
        return f"class name: {self.__class__}"


    def __repr__(self):
        return "hello"
pylint example.py --disable=all --enable=too-few-public-methods

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

@DanielNoord
Copy link
Collaborator

At least in terms of the current Pylint logic, I think that isn't correct. See [this comment ](https://github.com/pylint-

Wow.. I guess I would consider that a bug 😄 Either way, I'm against determining inclusion based on some heuristic of "non-trivialness". Whether all other dunder methods should be included is something for another issue I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants