Skip to content
This repository has been archived by the owner on Jul 18, 2024. It is now read-only.

Fix custom field support and add include_from_annotations. #54

Merged

Conversation

phbernardes
Copy link
Contributor

Hi,

This PR will:

  • Add support to include_from_annotations as shown in README and test_include_from_annotations.
  • Fix the error shown in test_multiple_level_relations where a custom field (user_cache in this example) is present in the Schema.

Thank you.

@jordaneremieff
Copy link
Owner

Hi @phbernardes, thanks for the PR!

I think this is a good idea, but prefer to handle the include from annotations case without introducing a new configuration flag. Currently a config error will be raised if both include and exclude are defined, but the behavior in the proposed changes would be inconsistent with that expectation. We could solve this by introducing additional validations to check when both the flag and include or exclude options are provided, but even still I think we ought to keep the amount of custom Djantic options in the config minimal.

What do you think about something like a special "__annotations__" string that could be set as the include to implement this behaviour?

For example:

class ProfileSchema(ModelSchema):
    
    website: str

    class Config:
        model = Profile
        include = "__annotations__"

@phbernardes phbernardes force-pushed the fix-custom-fields-support branch from 5b2e8da to 3a15978 Compare April 10, 2022 08:11
@phbernardes
Copy link
Contributor Author

Hi @phbernardes, thanks for the PR!

I think this is a good idea, but prefer to handle the include from annotations case without introducing a new configuration flag. Currently a config error will be raised if both include and exclude are defined, but the behavior in the proposed changes would be inconsistent with that expectation. We could solve this by introducing additional validations to check when both the flag and include or exclude options are provided, but even still I think we ought to keep the amount of custom Djantic options in the config minimal.

What do you think about something like a special "__annotations__" string that could be set as the include to implement this behaviour?

For example:

class ProfileSchema(ModelSchema):
    
    website: str

    class Config:
        model = Profile
        include = "__annotations__"

Hi @jordaneremieff, thanks for this review. Good point, implemented it in 3a15978.

@jordaneremieff jordaneremieff merged commit 7321b54 into jordaneremieff:main Apr 10, 2022
@phbernardes
Copy link
Contributor Author

@jordaneremieff, thank you for merging this. Can you please create a release?

@jordaneremieff
Copy link
Owner

@phbernardes sure, will do this week.

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

Successfully merging this pull request may close these issues.

2 participants