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

Update pydantic model Config classes #945

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

adamchainz
Copy link
Contributor

Update Schema class and subclasses for the pydantic config changes from its version 2.

I stopped part way since I realized the changes affect the API. Do you think using model_config makes sense for subclasses, or would you go with another design like ModelSchema.Meta?

@vitalik
Copy link
Owner

vitalik commented Nov 22, 2023

Hi @adamchainz

yeah, I'm still thinking about this... (see also discussion here #934 )

basically Pydantic wants to put stuff to model_config, while Django people are very familiar with class Meta

maybe the most optional solution would be to allow users to set pydantic settings in Meta and then copy them to model_config on class creation

@jkgenser
Copy link

jkgenser commented Dec 5, 2023

I think there should be minimal django ninja for wrapping pydantic and use the pydantic primitive. Otherwise users need to know django-ninja + pydantic. But on average users are going to be much more familiar with pydantic as it's widely used, I like @adamchainz approach

@vitalik
Copy link
Owner

vitalik commented Dec 5, 2023

@jkgenser

but what do you mean by adamchainz's approach ?

to have all handled by Meta:

class SomeData(ModelSchema):
     class Meta:
           model = User
           fields = '__all__'
           alias_generator=to_camel # < this is a pydantic setting

or split with model_config:

class SomeData(ModelSchema):
     model_config = ConfigDict(alias_generator=to_camel)
     class Meta:
           model = User
           fields = '__all__'

@KlemenS189
Copy link

My two cents here. I dislike Meta class approach in django. It's quite common that you have a BaseClass with it's own Meta class. Then you create a custom model/schema and inherit the BaseClass. If you then create a new Meta class in this new model, it's quite easy to forget inheritance of meta class from parent.

Example:

class BaseSchema(Schema):
    field: str
    
    class Meta:
        config = ConfigDict(populate_by_name=True, alias_generator=to_camel)


class UserSchema(BaseSchema):
    name: str
    class Meta:
        __fields__ = ["name"]

In this case, the Meta class in BaseSchema will be ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants