FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: reduce and update methods for DocumentArray and BaseDocument by JoanFM · Pull Request #1076 · docarray/docarray · GitHub

feat: reduce and update methods for DocumentArray and BaseDocument - #1076

Merged
JoanFM merged 12 commits into
feat-rewrite-v2from
feat-reduce-v2
Feb 6, 2023
Merged

feat: reduce and update methods for DocumentArray and BaseDocument#1076
JoanFM merged 12 commits into
feat-rewrite-v2from
feat-reduce-v2

Conversation

JoanFM commented Feb 2, 2023

Copy link
Copy Markdown
Member

Goals:

Implement reduce methods in utils

Joan Fontanals Martinez added 4 commits February 1, 2023 19:08
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
Comment thread docarray/utils/reduce.py Outdated
Comment thread docarray/utils/reduce.py Outdated
Comment thread docarray/utils/reduce.py Outdated
Comment thread docarray/utils/reduce.py Outdated
Comment thread docarray/utils/reduce.py Outdated
Comment thread docarray/utils/reduce.py Outdated
Comment thread docarray/utils/reduce.py Outdated
Comment thread docarray/utils/reduce.py Outdated
Comment thread docarray/utils/reduce.py Outdated
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
Comment thread docarray/utils/reduce.py Outdated
Comment thread docarray/base_document/document.py Outdated
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
Signed-off-by: Joan Fontanals Martinez <joan.martinez@jina.ai>
JoanFM changed the title feat: Implement reduce methods in utils feat: reduce and update methods for docarray and document Feb 3, 2023
JoanFM changed the title feat: reduce and update methods for docarray and document feat: reduce and update methods for DocumentArray and BaseDocument Feb 3, 2023
JoanFM requested a review from samsja February 3, 2023 11:46
Comment thread docarray/base_document/document.py Outdated
Comment on lines +130 to +133
elif (
isinstance(field_type, _GenericAlias)
and field_type.__origin__ is list
):

samsja Feb 3, 2023
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I am wondering if this will work with newer python version where you can pass list instead of List as a valid type hint ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

what Python version are u referring to?

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

in python 3.9 you can do

class A:
   b: list[inŧ]

instead of

class A:
   b: List[int]

I am almost sure they have maintain compatibility with older version but the fact that we are using a private object from the typing library tell me that we are not 100% sure we will support this new way.

Maybe there is a way not to use the _GenericAlias since it is private ?

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Oh actually we can use

from typing_inspect import is_generic_type

we already rely on this library for some type hint things and they make sure that their function are compatible with every python version

https://github.com/ilevkivskyi/typing_inspect

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

can u propose a PR on top of mine to do this? I am not sure what u are intending to tell. I did trial and error until I found the working formula

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I pushed a commit

Signed-off-by: samsja <sami.jaghouar@hotmail.fr>
Signed-off-by: samsja <sami.jaghouar@hotmail.fr>
JoanFM requested a review from samsja February 6, 2023 09:12

github-actions Bot commented Feb 6, 2023

Copy link
Copy Markdown

📝 Docs are deployed on https://ft-feat-reduce-v2--jina-docs.netlify.app 🎉

JoanFM merged commit 149ee5e into feat-rewrite-v2 Feb 6, 2023
JoanFM deleted the feat-reduce-v2 branch February 6, 2023 09:48
samsja mentioned this pull request Feb 6, 2023
47 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL