Skip to content

fix: fix dynamic class creation with doubly nested schemas #1747

Merged
samsja merged 3 commits intomainfrom
fix-create-from-schema
Aug 10, 2023
Merged

fix: fix dynamic class creation with doubly nested schemas #1747
samsja merged 3 commits intomainfrom
fix-create-from-schema

Conversation

@alaeddine-13
Copy link
Member

@alaeddine-13 alaeddine-13 commented Aug 10, 2023

the following case results in a KeyError:

from docarray import BaseDoc
from docarray.utils.create_dynamic_doc_class import create_base_doc_from_schema


class Nested2(BaseDoc):
    value: str


class Nested1(BaseDoc):
    nested: Nested2


class RootDoc(BaseDoc):
    nested: Nested1


new_my_doc_cls = create_base_doc_from_schema(RootDoc.schema(), 'RootDoc')

the reason is, create_base_doc_from_schema will keep referring to definitions field of the traversed schema when creating its corresponding model. However, the definitions field only exists for the root schema

@alaeddine-13 alaeddine-13 requested a review from samsja August 10, 2023 11:40
@alaeddine-13
chore: linting
bc3e1d6 
Signed-off-by: Alaeddine Abdessalem <[email protected]>
@alaeddine-13 alaeddine-13 force-pushed the fix-create-from-schema branch from a0a81a3 to bc3e1d6 Compare August 10, 2023 11:49
@github-actions
Copy link

github-actions bot commented Aug 10, 2023

📝 Docs are deployed on https://ft-fix-create-from-schema--jina-docs.netlify.app 🎉

@codecov
Copy link

codecov bot commented Aug 10, 2023 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: +1.39% 🎉

Comparison is base (691d939) 83.60% compared to head (bc3e1d6) 85.00%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1747      +/-   ##
==========================================
+ Coverage   83.60%   85.00%   +1.39%     
==========================================
  Files         134      134              
  Lines        8838     8842       +4     
==========================================
+ Hits         7389     7516     +127     
+ Misses       1449     1326     -123
Flag Coverage Δ
docarray 85.00% <100.00%> (+1.39%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
docarray/utils/create_dynamic_doc_class.py 83.67% <100.00%> (+0.69%) ⬆️

... and 11 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@samsja samsja merged commit adb0d01 into main Aug 10, 2023
@samsja samsja deleted the fix-create-from-schema branch August 10, 2023 13:28
@alaeddine-13 alaeddine-13 mentioned this pull request Aug 22, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samsja samsja samsja approved these changes

Assignees

No one assigned

Labels

area/core area/testing size/s

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@alaeddine-13 @samsja