Skip to content

[BUG] PatchDict errors with inherited schemas #1324

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

Open
filippomc opened this issue Oct 22, 2024 · 7 comments
Open

[BUG] PatchDict errors with inherited schemas #1324

filippomc opened this issue Oct 22, 2024 · 7 comments

Comments

@filippomc
Copy link

filippomc commented Oct 22, 2024

Describe the bug

I have a schema hierarchy such as:

class ViewableContent(Schema):
    name: str
    description: str = None

class MySchema(ViewableContent):
   other: str # If I don't add a new field the problem does not arise

Then add a router like the following:

@router.patch('/{uuid}', response={200: MySchema})
@transaction.atomic
def my_update(request: HttpRequest, uuid: str, payload: PatchDict[MySchema]):
   ...

When I run my application the following error is raised:

  File "/home/user/mnp/applications/neuroglass-research/backend/neuroglass_research/api/__init__.py", line 4, in <module>
    from .studies import router as studies_router
  File "/home/user/mnp/applications/neuroglass-research/backend/neuroglass_research/api/studies.py", line 69, in <module>
    def update_study(request: HttpRequest, study_id: int, payload: PatchDict[UpdateStudyPayload]):
                                                                   ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^
  File "/home/user/miniconda3/envs/mnp/lib/python3.12/site-packages/ninja/patch_dict.py", line 45, in __getitem__
    new_cls = create_patch_schema(schema_cls)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/user/miniconda3/envs/mnp/lib/python3.12/site-packages/ninja/patch_dict.py", line 29, in create_patch_schema
    t = schema_cls.__annotations__[f]
        ~~~~~~~~~~~~~~~~~~~~~~~~~~^^^
KeyError: 'name'

Versions:

  • Python version: 3.12
  • Django version: 5.1.2
  • Django-Ninja version: 1.3.0
  • Pydantic version: 2.9.2
@filippomc
Copy link
Author

filippomc commented Oct 22, 2024

Possibly related discussion: pydantic/pydantic#4242

@vitalik
Copy link
Owner

vitalik commented Oct 22, 2024

If I don't add a new field the problem does not arise

so problem only appears when you ADD some field ? or always ?

@filippomc
Copy link
Author

If I don't add a field it's like Pydantic is considering the two classes equivalent and what I see is that the annotations are the same from the base class. When adding a field, that field is the only one within the annotations dict.

@vitalik
Copy link
Owner

vitalik commented Oct 23, 2024

well I still do not understand you.. can you show two examples working and non working or something...

@filippomc
Copy link
Author

filippomc commented Oct 24, 2024

The not working example is the one I originally posted, and the application breaks on any path with the error above.
These are a few ones that are working for me:

Working -- no inheritance:

class MySchema(Schema):
   name: str
   description: str = None
   other: str

Working -- duplicate all base class fields

class ViewableContent(Schema):
   name: str
   description: str = None

class MySchema(ViewableContent):
   name: str = None
   description: str = None
   other: str 

Working -- subclassing but no new fields

class ViewableContent(Schema):
   name: str
   description: str = None

class MySchema(ViewableContent):
   pass

Created an example project with all the examples

@EduardoCastanho
Copy link

EduardoCastanho commented Jan 13, 2025

Hi

I have encountered the same bug with inherited Schemas

The exception originates from the following method, where it uses the __annotations__ to decide if the field type needs to become Optional :

def create_patch_schema(schema_cls: Type[Any]) -> Type[ModelToDict]:
values, annotations = {}, {}
for f in schema_cls.__fields__.keys():
t = schema_cls.__annotations__[f]
if not is_optional_type(t):
values[f] = getattr(schema_cls, f, None)
annotations[f] = Optional[t]
values["__annotations__"] = annotations
OptionalSchema = type(f"{schema_cls.__name__}Patch", (schema_cls,), values)
class OptionalDictSchema(ModelToDict):
_wrapped_model = OptionalSchema
_wrapped_model_dump_params = {"exclude_unset": True}
return OptionalDictSchema

In line 29 sometimes it cannot find the root Schema fields, I think is related with how python lazy-create and resolves __annotations__

https://docs.python.org/3/howto/annotations.html#accessing-the-annotations-dict-of-an-object-in-python-3-10-and-newer

If no field is added to leaf Schema/Class no __annotations__ are created and when it is accessed the root __annotations__ is returned with all root Schema Fields.

If a field or more are added to leaf Schema/Class then the created __annotations__ is not an intersection between root a leaf class field, but only leaf Schema fields (among other stuff…), failing to find root Schema fields,.

In my project, I temporarily fix the issue using the suggest approach to get fields types in this pydantic discussion:
pydantic/pydantic#2360

from typing import get_type_hints
//(...)
def fix_create_patch_schema(schema_cls: Type[Any]) -> Type[ModelToDict]:
    values, annotations = {}, {}
    schema_cls_type_hints = get_type_hints(schema_cls)
    for f in schema_cls.__fields__.keys():
        t = schema_cls_type_hints[f]
        if not is_optional_type(t):
            values[f] = getattr(schema_cls, f, None)
            annotations[f] = Optional[t]
    values["__annotations__"] = annotations
    OptionalSchema = type(f"{schema_cls.__name__}Patch", (schema_cls,), values)

    class OptionalDictSchema(ModelToDict):
        _wrapped_model = OptionalSchema
        _wrapped_model_dump_params = {"exclude_unset": True}

    return Body[OptionalDictSchema]

Given my lack of knowledge about the Django-ninja structure.

Do you think is a good solution ? Should I make a PR ?

@iStorry
Copy link

iStorry commented Mar 10, 2025

Any update on this?

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

No branches or pull requests

4 participants