Skip to content

feat: type implement protobuf#806

Merged
JohannesMessner merged 1 commit into
feat-rewrite-v2from
feat-type-implement-protobuf
Nov 21, 2022
Merged

feat: type implement protobuf#806
JohannesMessner merged 1 commit into
feat-rewrite-v2from
feat-type-implement-protobuf

Conversation

@dongxiang123

@dongxiang123 dongxiang123 commented Nov 17, 2022

Copy link
Copy Markdown
Contributor

Goals:

Atm the docarray types implement _to_nested_item_protobuf. Which basically dump the types into the correct proto object. This allow the to_protobuf method of Document to be quite short. We need to do the same for the from_protobuf

Therefore at the BaseNode interface we need to implement a _from_nested_item_protobuf which take a proto message as input initialize the node. Document, DocumentArray, and Types (all of the possible nodes) should define this method. For Document and DocumentArray this is quite straightforward. For Types we basically need to break the if statement from from_protobuf into each of the type.

@dongxiang123 dongxiang123 force-pushed the feat-type-implement-protobuf branch from cef7c45 to c0430fd Compare November 17, 2022 08:35
@dongxiang123 dongxiang123 changed the base branch from main to feat-rewrite-v2 November 17, 2022 08:36
@dongxiang123 dongxiang123 force-pushed the feat-type-implement-protobuf branch from c0430fd to 5461812 Compare November 17, 2022 09:19
@JohannesMessner

Copy link
Copy Markdown
Member

@dongxiang123 could you make sure that all the pre-commit "things" (lack, ruff, mypy, etc) are applied?

Comment thread docarray/typing/chunks.py Outdated
Comment thread docarray/typing/nested.py Outdated
Comment thread docarray/typing/text.py Outdated
@dongxiang123 dongxiang123 force-pushed the feat-type-implement-protobuf branch 4 times, most recently from ce61c4d to 7bb5d25 Compare November 18, 2022 03:48
@dongxiang123 dongxiang123 marked this pull request as ready for review November 18, 2022 03:56
@dongxiang123 dongxiang123 force-pushed the feat-type-implement-protobuf branch 13 times, most recently from 251d16b to 75c87db Compare November 21, 2022 08:12

@samsja samsja left a comment

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.

minor comments

Comment thread docarray/typing/id.py
Comment thread docarray/typing/tensor/torch_tensor.py
* refactor(v2): renamed variable and files

Signed-off-by: Sami Jaghouar <sami.jaghouar@hotmail.fr>

* refactor(v2): renamd test

Signed-off-by: Sami Jaghouar <sami.jaghouar@hotmail.fr>

Signed-off-by: Sami Jaghouar <sami.jaghouar@hotmail.fr>

feat: add read from proto
@dongxiang123 dongxiang123 force-pushed the feat-type-implement-protobuf branch from 3771efd to 26aae10 Compare November 21, 2022 09:56
@samsja samsja changed the title Feat type implement protobuf feat: type implement protobuf Nov 21, 2022

@samsja samsja left a comment

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.

lgtm, nice refactoring !

@JohannesMessner JohannesMessner merged commit 1420385 into feat-rewrite-v2 Nov 21, 2022
@JohannesMessner JohannesMessner deleted the feat-type-implement-protobuf branch November 21, 2022 10:06
dongxiang123 added a commit that referenced this pull request Nov 21, 2022
* refactor(v2): renamed variable and files

Signed-off-by: Sami Jaghouar <sami.jaghouar@hotmail.fr>

* refactor(v2): renamd test

Signed-off-by: Sami Jaghouar <sami.jaghouar@hotmail.fr>

Signed-off-by: Sami Jaghouar <sami.jaghouar@hotmail.fr>

feat: add read from proto

Co-authored-by: samsja <55492238+samsja@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

3 participants