-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
support self argument #632
Conversation
9f6c4f1
to
85ea317
Compare
@samuelcolvin so that would work if 'self' is at the root of the dict but if it's nested it doesn't. from pydantic import BaseModel
class Nested(BaseModel):
self: str
class Model(BaseModel):
nested: Nested
m = Model.parse_obj({"nested": {"self": "me"}})
#pydantic.error_wrappers.ValidationError: 1 validation error
#nested
# __init__() got multiple values for argument 'self' (type=type_error) |
That seems to do it. |
Codecov Report
@@ Coverage Diff @@
## master #632 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 15 15
Lines 2651 2632 -19
Branches 524 516 -8
=====================================
- Hits 2651 2632 -19 |
Note that this breaks nested models that overload a BaseModel's |
Humm, hadn't thought of that. Anything we can do? Or just add a note to the release notes. |
If I understand the issue correctly, the first argument in the
You could even to rename it to something like I think supporting canonical init overloading is relatively important. The |
This commit enables nested model `__init__` statements to be executed while still allowing `self` as an argument. Effectively reverses the changes from pydantic#632 while still enabling the feature it implemented. In theory, there will still be a collision if someone ever tried to use `pydantic_base_model/settings_init` as an arg, but I don't know how to engineer a case where a collision would *never* happen, I'm not sure there is one. This commit also added a test for both BaseModel` and `BaseSettings` for both the `self`-as-a-parameter and the nested `__init__` features since `BaseSettings` now has the same issue as `BaseModel` since it invoked an `__init__` with self. I have added a comment under the `__init__` for both `BaseModel` and `BaseSetting` since not having `self` as the first arg is such a rarity within Python that it will likely confuse future developers who encounter it. The actual name of the variable referencing the class itself can be up for debate.
This commit enables nested model `__init__` statements to be executed while still allowing `self` as an argument. Effectively reverses the changes from pydantic#632 while still enabling the feature it implemented. In theory, there will still be a collision if someone ever tried to use `pydantic_base_model/settings_init` as an arg, but I don't know how to engineer a case where a collision would *never* happen, I'm not sure there is one. This commit also added a test for both BaseModel` and `BaseSettings` for both the `self`-as-a-parameter and the nested `__init__` features since `BaseSettings` now has the same issue as `BaseModel` since it invoked an `__init__` with self. I have added a comment under the `__init__` for both `BaseModel` and `BaseSetting` since not having `self` as the first arg is such a rarity within Python that it will likely confuse future developers who encounter it. The actual name of the variable referencing the class itself can be up for debate.
* Re-enable nested model init calls while still allowing self This commit enables nested model `__init__` statements to be executed while still allowing `self` as an argument. Effectively reverses the changes from #632 while still enabling the feature it implemented. In theory, there will still be a collision if someone ever tried to use `pydantic_base_model/settings_init` as an arg, but I don't know how to engineer a case where a collision would *never* happen, I'm not sure there is one. This commit also added a test for both BaseModel` and `BaseSettings` for both the `self`-as-a-parameter and the nested `__init__` features since `BaseSettings` now has the same issue as `BaseModel` since it invoked an `__init__` with self. I have added a comment under the `__init__` for both `BaseModel` and `BaseSetting` since not having `self` as the first arg is such a rarity within Python that it will likely confuse future developers who encounter it. The actual name of the variable referencing the class itself can be up for debate. * Please Lint * Please Black * Update from comments
fix #629
Checklist
Documentation reflects the changes where applicableHISTORY.rst
has been updated#<number>
@<whomever>