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

do not report too-few-public-methods for attr.s, attr.dataclass, typing.NamedTuple and similar. #3732

Open
graingert opened this issue Jul 10, 2020 · 13 comments
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Hacktoberfest Help wanted 🙏 Outside help would be appreciated, good for new contributors High priority Issue with more than 10 reactions Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@graingert
Copy link
Contributor

graingert commented Jul 10, 2020

do not report too-few-public-methods for attr.s, attr.dataclass, typing.NamedTuple and similar.

example:

import attr

@attr.dataclass(frozen=True)  # pylint-ignore: too-few-public-methods
class Foo:
    ham: str
    spam: str

@uriva As mentioned by @matkoniecz in most cases the check makes sense. If your class has a single method on it, it's probably a function in disguise. You can also configure the number of public methods with min-public-methods. But there are a couple of classes for which we should not report this, as it makes perfect sense to not have too many methods or no methods at all. This includes attrs, dataclasses, typing.NamedTuple and similar.

Originally posted by @PCManticore in #2710 (comment)

@uriva
Copy link

uriva commented Jul 10, 2020

I'm not sure why this reopened, I am no longer using pylint (moved to flake8) so no interest in this anymore.
Thanks

@matkoniecz
Copy link

matkoniecz commented Jul 10, 2020

I am also not sure why you copied the comment outright pinging all involved rather than posting only relevant part.

Posting "I'm going to close this issue" while opening an issue is bizarre.

Also: give a code sample wrongly reported as mistake.

@graingert
Copy link
Contributor Author

@matkoniecz comment now quoted, code example added

@graingert
Copy link
Contributor Author

I am also not sure why you copied the comment outright pinging all involved rather than posting only relevant part.

I just hit the "reference in new issue button"

@Pierre-Sassoulas
Copy link
Member

This message is probably going to be disabled by default because there is a lot of example of place where it makes sense to have few public method (Django Meta class, inheritance of class that implements a run() function etc.) See #3512 . If you think this is useful and want to use it, but with a whitelist of class that can have few public method, I don't think there is an easy way to do that right now, but I think this is doable.

@earonesty
Copy link

earonesty commented Dec 26, 2020

Configurable suppressions for classes would be generally useful:

Simple config override in .pylintrc for classes that:

# [CLASSRULES.<CUSTOM_NAME>]
# metaclass_match=<metaclass_regex>
# parent_match=<parent_regex>
# disable=list,of,disables

Would solve a lot of these issues.

marmarek added a commit to WillyPillow/qubes-core-admin-client that referenced this issue Feb 19, 2021
marmarek added a commit to WillyPillow/qubes-core-admin-client that referenced this issue Feb 19, 2021
@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Help wanted 🙏 Outside help would be appreciated, good for new contributors labels Feb 20, 2021
@Pierre-Sassoulas Pierre-Sassoulas added the Good first issue Friendly and approachable by new contributors label Mar 2, 2021
@Pierre-Sassoulas Pierre-Sassoulas pinned this issue May 11, 2021
@Pierre-Sassoulas
Copy link
Member

Also true for exceptions see : #4464

@wnoise
Copy link

wnoise commented May 15, 2021

Also true for anything inheriting from typing.Protocol.

@Pierre-Sassoulas Pierre-Sassoulas unpinned this issue Jul 1, 2021
@graingert
Copy link
Contributor Author

@Pierre-Sassoulas why was this unpinned?

@graingert
Copy link
Contributor Author

this should also not report: too-many-instance-attributes for attr.s

@Pierre-Sassoulas
Copy link
Member

I unpinned it because there was more pressing issues to advertise first (ranked according to number of thumbsup or heart reactions), it does not mean that I would not review a fix for this ;)

@g1itch
Copy link

g1itch commented Jul 29, 2021

What about context managers? May pylint detect them by __enter__() and __exit__() methods defined?

@Pierre-Sassoulas Pierre-Sassoulas added the Needs PR This issue is accepted, sufficiently specified and now needs an implementation label Jul 2, 2022
@davebirch
Copy link

If this is still for consideration, attrs now support an @define decorator, as well as @attr.s.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Good first issue Friendly and approachable by new contributors Hacktoberfest Help wanted 🙏 Outside help would be appreciated, good for new contributors High priority Issue with more than 10 reactions Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

8 participants