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
Teach mypy plugin that validator methods are classmethods #4102
Conversation
For your consideration: a patch which implements the suggestion I made here: pydantic#4101 Briefly: pydantic automatically wraps validator methods using `@classmethod`. Hence the first argument to a user-provided validator should be `cls`. But mypy doesn't know this: it analyses validator methods like any other regular method, believing the first parameter `cls` to be a model instance (usually denoted self). This means that if one annotates `cls` as `Type[...]` then mypy believes raises an error: error: The erased type of self "Type[temp.Model]" is not a supertype of its class "temp.Model" I concede that this is an extremely niche thing. The only tangible end-user benefit I can think of is that it'll stop you from calling instance methods in a validator. ---- I haven't written a mypy plugin before, so this is a bit of a hack-until-it-works. But it was more straightforward than I expected to get something working!
please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also add a failing example to the tests.
tests/mypy/modules/plugin_success.py
Outdated
name: str | ||
|
||
@validator('name') | ||
def noop_validator_with_annotations(cls: Type['ModelWithAnnotatedValidator'], name: str) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is wrong, surely (like self
) you can omit the type hint for cls
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh yes. I think I'd put this in so that the test failed without the change. Let me see if I can come up with something better.
otherwise LGTM. Please update. Also conflicts. |
Many thanks for reviewing. I've merged in master and updated the tests. Please review. |
thanks so much. |
For your consideration: a patch which implements the suggestion I made here:
#4101
Briefly: pydantic automatically wraps validator methods using
@classmethod
. Hence the first argument to a user-provided validatorshould be
cls
. But mypy doesn't know this: it analyses validatormethods like any other regular method, believing the first parameter
cls
to be a model instance (usually denoted self).This means that if one annotates
cls
asType[...]
then mypybelieves raises an error:
I concede that this is an extremely niche thing. The only tangible
end-user benefit I can think of is that it'll stop you from calling
instance methods in a validator.
I haven't written a mypy plugin before, so this is a bit of a
hack-until-it-works. But it was more straightforward than I expected to
get something working!
Change Summary
Related issue number
fix #4101
Checklist
changes/<pull request or issue id>-<github username>.md
file added describing change(see changes/README.md for details)