-
Notifications
You must be signed in to change notification settings - Fork 24
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
Add support for non-call @root_validator #114
Conversation
In Pydantic v1 having a plain decorator like `@root_validator` (without `()`) was common and worked fine. In v2, `@model_validator` needs a `mode` argument. Add a test and adjust implementation, so that the program replaces ```py @root_validator def foo(cls, values): ... ``` with: ```py @model_validator(mode="after") @classmethod def foo(cls, values): ... ```
Thanks 🙏 |
bump_pydantic/codemods/validator.py
Outdated
else: | ||
raise RuntimeError("decorator as attribute is not implemented yet.") # this should never happen ATM |
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.
We shouldn't have this conditional. Can you add a xfail test for which this would happen?
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.
OK. Removed RuntimeError
and moved the Name
case under else
.
Can you add a xfail test for which this would happen?
I think the test is already there in test_import_pydantic, no?
else: | ||
# Assuming node.decorator matches Name() | ||
# (once ROOT_VALIDATOR_DECORATOR supports Attribute() this needs a separate case). | ||
# Changing `@root_validator` (Name, not Call) into `@model_validator(...)` (Call) | ||
decorator = cst.Call(func=cst.Name(new_name), args=self._args) |
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.
But it shouldn't be an else, should it? Does the test you mention fails differently now?
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.
If I make it an elif
(like before) without the else
branch, mypy complains about decorator
potentially being unbound.
Does the test you mention fails differently now?
Well, the test fails the same everytime ATM, because we never reach that case due to what ROOT_VALIDATOR_DECORATOR
is matching (it's not matching decorators as attributes now, so those cases won't be processed even).
In Pydantic v1 having a plain decorator like
@root_validator
(without()
) was common and worked fine.In v2,
@model_validator
needs amode
argument.Add a test and adjust implementation, so that the program replaces
with:
The implementation maybe is not beautiful, but seems to work 😅.
While working on this I noticed that having a
@model_validator()
(without args) in v2 produces an error. One has to provide themode
arg.I can open a separate PR for this,
but this change most likely will need a separate
visit
methodfor
ROOT_VALIDATOR_DECORATOR
,because
mode
infield_validator
can be omitted.