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

Pydantic models should not trigger too-few-public-methods #3370

Closed
chaoflow opened this issue Jan 28, 2020 · 9 comments · Fixed by #5191
Closed

Pydantic models should not trigger too-few-public-methods #3370

chaoflow opened this issue Jan 28, 2020 · 9 comments · Fixed by #5191
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
Milestone

Comments

@chaoflow
Copy link

Is your feature request related to a problem? Please describe

Pylint triggers too-few-public-methods for pydantic models.

They are very similar to dataclasses which got special treatment in #3025

Describe the solution you'd like

Pylint should not emit too-few-public-methods for pydantic models or provide a mechanism so pydantic can declare this exception.

Additional context

@PCManticore
Copy link
Contributor

Thanks, at this point we can probably add an option to skip too-few-public-methods for certain data style classes, such as namedtuple, pydantic and dataclasses.

@PCManticore PCManticore added the Enhancement ✨ Improvement to a component label Jan 30, 2020
@chaoflow
Copy link
Author

That sounds great. Note that pydantic comes also with a dataclass implementation that supports validation. It might make sense to look into supporting this as well.

@NickKush
Copy link

NickKush commented Mar 2, 2021

Any updates on this issue? 🙂

@Olegt0rr
Copy link

Olegt0rr commented Apr 7, 2021

Please set "help wanted" tag to search for someone who can fix this issue

@Pierre-Sassoulas Pierre-Sassoulas added Good first issue Friendly and approachable by new contributors Help wanted 🙏 Outside help would be appreciated, good for new contributors labels Apr 7, 2021
@miketheman
Copy link
Contributor

miketheman commented Oct 15, 2021

I'm game to take a crack at this, but I'm uncertain if it's acceptable to add the pydantic package to the requirements_test.txt file for this test? Is there another place I should look for how external test deps are expressed?

@Pierre-Sassoulas
Copy link
Member

We're trying not to add packages to the tests requirements right now. I think at this point it could be a configurable option to add classes that should be ignored by this check. This way we could test with any classes even one we created specifically for the test and easily add other similar classes.

@miketheman
Copy link
Contributor

miketheman commented Oct 15, 2021

... it could be a configurable option to add classes that should be ignored by this check

I like this - something like this?

[DESIGN]
exclude-too-few-public-methods=pydantic.*

?

@Pierre-Sassoulas
Copy link
Member

Looks great ! How would we handle multiple values ? filter separated by comma, a regex ?

@miketheman
Copy link
Contributor

I guess following the existing patterns, it'd be a comma-separated list of module names/prefixes.

DanielNoord added a commit that referenced this issue Oct 26, 2021
Allow excluding classes based on their ancestors from the ``too-few-public-methods`` checker.

Closes #3370

Signed-off-by: Mike Fiedler <miketheman@gmail.com>

Co-authored-by: Daniël van Noord <13665637+DanielNoord@users.noreply.github.com>
@DanielNoord DanielNoord added this to the 2.12.0 milestone Oct 26, 2021
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants