Skip to content

fix: fix to dict exclude#1481

Merged
JoanFM merged 6 commits intomainfrom
fix-to-dict
May 2, 2023
Merged

fix: fix to dict exclude#1481
JoanFM merged 6 commits intomainfrom
fix-to-dict

Conversation

@samsja
Copy link
Copy Markdown
Member

@samsja samsja commented May 2, 2023 •
edited by JoanFM
Loading

Context

this PR fix calling dict on a document that contain a nested DocList.

Now when calling dict with a nested DocList the dict results will be a list of dict whereas before it was just the DocList.

Fix the real problem that was hidden under this issue #1474

class SimpleDoc(BaseDoc):
    simple_tens: NdArray[10]

class NestedDoc(BaseDoc):
    docs: DocList[SimpleDoc]
    hello: str = 'world'

nested_docs = NestedDoc(
    docs=DocList[SimpleDoc]([SimpleDoc(simple_tens=np.ones(10)) for j in range(2)]),
) 

nested_docs.dict() # use to fail now is working

@samsja samsja marked this pull request as ready for review May 2, 2023 11:33
@samsja samsja requested review from JoanFM and JohannesMessner May 2, 2023 11:33
@samsja
fix: fix mypy
4932157 
Signed-off-by: samsja <[email protected]>
@JoanFM
Copy link
Copy Markdown
Member

JoanFM commented May 2, 2023

Can u add some examples of how it looked before and after?

JoanFM
JoanFM requested changes May 2, 2023
View reviewed changes
samsja added 3 commits May 2, 2023 15:48
JoanFM
JoanFM requested changes May 2, 2023
View reviewed changes
docarray/base_doc/doc.py
exclude_defaults: bool = False,
exclude_none: bool = False,
) -> 'DictStrAny':
"""
Copy link
Copy Markdown
Member

@JoanFM JoanFM May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add description of each argument?

@JoanFM JoanFM linked an issue May 2, 2023 that may be closed by this pull request
Bug: validation of DocList inside a Doc raises error #1474
Closed
@github-actions
Copy link
Copy Markdown

github-actions bot commented May 2, 2023

📝 Docs are deployed on https://ft-fix-to-dict--jina-docs.netlify.app 🎉

@JoanFM JoanFM merged commit e37a59e into main May 2, 2023
@JoanFM JoanFM deleted the fix-to-dict branch May 2, 2023 17:15
@samsja samsja mentioned this pull request May 3, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JoanFM JoanFM JoanFM approved these changes

@JohannesMessner JohannesMessner Awaiting requested review from JohannesMessner

Assignees

No one assigned

Labels

area/core area/testing size/m

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: validation of DocList inside a Doc raises error

2 participants

@samsja @JoanFM