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

Model with field name that shadows existing BaseModel attribute leads to unexpected behavior #242

Closed
nabla-c0d3 opened this Issue Aug 3, 2018 · 10 comments

Comments

Projects
None yet
3 participants
@nabla-c0d3
Copy link
Contributor

nabla-c0d3 commented Aug 3, 2018

Hello,
First of all this is an awesome project, I want to use it in all my tools :).

I ran into a small issue: when defining a model that has a field with the same name as one of BaseModel's existing attributes/methods, the field gets parsed but cannot be retrieved:

class BadModel(BaseModel):
    schema: str

obj = BaseModel(**{'schema': 'abc'})
print(obj.schema)

This prints <bound method BaseModel.schema of <class 'pydantic.main.BaseModel'>> instead of abc.

This makes perfect sense when looking at the code but is a bit unexpected. To be honest, I'm not sure what the "right" behavior should be here?

@samuelcolvin

This comment has been minimized.

Copy link
Owner

samuelcolvin commented Aug 3, 2018

Probably best to raise an exception?

@nabla-c0d3

This comment has been minimized.

Copy link
Contributor Author

nabla-c0d3 commented Aug 3, 2018

I thought of it but it would have been painful in my case.

I am parsing an OpenAPI 3.0 document(https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.0.md#parameterObject) so I do not control the format.

Not being able to use pydantic for this would be annoying. My current workaround is to expose the schema field as schema_.

@samuelcolvin

This comment has been minimized.

Copy link
Owner

samuelcolvin commented Aug 3, 2018

You can use aliases so the external property Mgmt can be different.

@nabla-c0d3

This comment has been minimized.

Copy link
Contributor Author

nabla-c0d3 commented Aug 3, 2018

FYI I had a similar problem with an OpenAPI field called in, which is a reserved keyword in Python. I did a similar workaround to expose it as in_ (and use dynamic schema creation to define the field).

@nabla-c0d3

This comment has been minimized.

Copy link
Contributor Author

nabla-c0d3 commented Aug 3, 2018

Oh thanks, let me look into aliases. If that works then an exception seems fine.

@nabla-c0d3

This comment has been minimized.

Copy link
Contributor Author

nabla-c0d3 commented Aug 3, 2018

Yeah alisases work well. Thanks! In this case an exception would be fine (with perhaps a hint to use an alias).

@samuelcolvin

This comment has been minimized.

Copy link
Owner

samuelcolvin commented Aug 3, 2018

Would you like to submit a pr?

@nabla-c0d3

This comment has been minimized.

Copy link
Contributor Author

nabla-c0d3 commented Aug 3, 2018

I will try...

@layday

This comment has been minimized.

Copy link
Contributor

layday commented Aug 4, 2018

schema is a class method. Just relocate it to the metaclass along with every other classmethod (which would also mean you can make it a property).

In [1]: import pydantic

In [2]: class Model(pydantic.BaseModel):
   ...:     schema: str

In [3]: Model.schema
Out[3]: 
{'title': 'Model',
 'type': 'object',
 'properties': {'schema': {'title': 'Schema',
   'required': True,
   'type': 'str'}}}

In [4]: Model(schema='a schema').schema
Out[4]: 'a schema'
@samuelcolvin

This comment has been minimized.

Copy link
Owner

samuelcolvin commented Aug 4, 2018

I think it will confuse people a lot of methods and/or attributes change between the class and the instance.

Eg. Model.schema does one thing but model.schema does something completely different.

Better to stick to classmethods and raise an exception if fields are setup with any of their names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.