Skip to content

fix: fix to dict exclude#1481

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

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

Conversation

@samsja

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

Copy link
Copy Markdown
Member

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
fix: fix to dict exclude
77084f5 
Signed-off-by: samsja <sami.jaghouar@hotmail.fr>
@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 <sami.jaghouar@hotmail.fr>
@JoanFM

JoanFM commented May 2, 2023

Copy link
Copy Markdown
Member

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

JoanFM
JoanFM requested changes May 2, 2023
View reviewed changes
Comment thread tests/units/document/test_base_document.py Outdated
samsja added 3 commits May 2, 2023 15:48
@samsja
fix: rename test
d51a253 
Signed-off-by: samsja <sami.jaghouar@hotmail.fr>
@samsja
fix: fixx isinstance check
df2489f 
Signed-off-by: samsja <sami.jaghouar@hotmail.fr>
@samsja
fix: fix exclude bug
29f469d 
Signed-off-by: samsja <sami.jaghouar@hotmail.fr>
JoanFM
JoanFM requested changes May 2, 2023
View reviewed changes
Comment thread docarray/base_doc/doc.py Outdated
Comment thread docarray/base_doc/doc.py
exclude_defaults: bool = False,
exclude_none: bool = False,
) -> 'DictStrAny':
"""

@JoanFM JoanFM May 2, 2023

Copy link
Copy Markdown
Member

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
docs: update docarray/base_doc/doc.py
d09e33d 
Signed-off-by: Joan Fontanals <jfontanalsmartinez@gmail.com>
@github-actions

github-actions Bot commented May 2, 2023

Copy link
Copy Markdown

📝 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