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

[nabla-c0d3]

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]

Probably best to raise an exception?

[nabla-c0d3]

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]

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

[nabla-c0d3]

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]

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

[nabla-c0d3]

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

[samuelcolvin]

Would you like to submit a pr?

[nabla-c0d3]

I will try...

[layday]

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]

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.

[mreschke]

I am a bit surprised this PR made it into pydantic. Think about building an ORM. Imagine all of your pydantic models (like Users in the example below) extend your own base Model class which had classmethods of find(), where(), and get(). This allows you to use your model in an ORM fasion like so.

• Users.find(1)
• Users.where('field', 'value').get()
• And many others (orderBy, limit etc...)

All fine, until the user wants a field called find, or where, or get etc... I understand they can set the field to get_ and use a alias for the actual db column named get but this seems an ugly hack when you could have just allowed shadowing.

With the above PR utils.validate_field_name will throw a shadow error if the user wants fields called find or where or get etc... Your main argument is that it's "confusing" the difference between Users.find and user.find. Well, they are two completely different things, and should be allowed. Shadowing is an important part of any language and is a powerful tool when dealing with class variables vs instance variables. My only option at this point is to maintain a fork or pydantic which removes this validate_field_name method completely so I can build a nice ORM without working about stomping all over the users "field" namespace.

[mreschke]

Perhaps allow shadowing could be a config variable? That would be helpful.

[layday]

See #1001.

[mreschke]

@layday thanks I have seen that interesting thread. But that discussion is more around public instance variables as opposed to class variable vs instance variable shadowing. Obviously you wouldn't want to remove the utils.validate_field_name at this point since its been there since 0.12. But if we can pass in config: Type['BaseConfig'] and make it a config property so we can control the shadow behavior from a config that would be a flexible solution.

[layday]

Well, that kind of shadowing isn't compatible with type checking in Python; you cannot have a field with a disparate type on a sub-model. You can define your class methods on the metaclass so that they are not present on instances of Users but your type checker (assuming you use one) won't know what to do with them either:

from pydantic.main import ModelMetaclass, BaseModel


class UsersMeta(ModelMetaclass):
    def find(cls, pk: int) -> 'Users': ...


class Users(BaseModel, metaclass=UsersMeta):
    find: int

Users.find  # Should this be `int` or `(pk: int) -> Users`?

Anyway, this will work without requiring any changes to Pydantic, assuming that you don't step on any of its own methods.