Skip to content

feat: add validate function to documents to have less verbose api#1058

Merged
samsja merged 7 commits intofeat-rewrite-v2from
feat-validate-docs
Jan 27, 2023
Merged

feat: add validate function to documents to have less verbose api#1058
samsja merged 7 commits intofeat-rewrite-v2from
feat-validate-docs

Conversation

@samsja
Copy link
Copy Markdown
Member

@samsja samsja commented Jan 26, 2023

Signed-off-by: samsja [email protected]

Comtext

Follow up on this : DocArray backlog

    class MyDoc(BaseDocument):
        text1: Text
        text2: Text

    MyDoc(text1='hello', text2=Text(text='world'))

allow to pass directly a Text

Copy link
Copy Markdown
Member

@JohannesMessner JohannesMessner left a comment

Choose a reason for hiding this comment

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

Like it, but I think we should do the same for all of our default docs: identify the main content field and allow that to be passed directly. I think for most of the other ones that would be a tensor instead of str

@samsja
Copy link
Copy Markdown
Member Author

samsja commented Jan 26, 2023

yes I will add the resti n this pr as well

for the other document what do we do

if it is a string we pass to url ?
if it is a tensor we pass to tensor field ?

@anna-charlotte what do u think ?

@JohannesMessner
Copy link
Copy Markdown
Member

yes I will add the resti n this pr as well

for the other document what do we do

if it is a string we pass to url ? if it is a tensor we pass to tensor field ?

@anna-charlotte what do u think ?

So instead of identifying one main content field per predefined doc we decide based on the input data's type? Could also make sense, but lead to confusion if multiple fields have the same type. E.g. some Document might have two fields of type str, then we need to choose one of those

@samsja
Copy link
Copy Markdown
Member Author

samsja commented Jan 26, 2023

So instead of identifying one main content field per predefined doc we decide based on the input data's type? Could also make sense, but lead to confusion if multiple fields have the same type. E.g. some Document might have two fields of type str, then we need to choose one of those

but we do this only for the one that make sense. But nothing automatic

@anna-charlotte
Copy link
Copy Markdown
Contributor

Yes, I like it, too, especially with `Text(text='some text') its too verbose, with the others I don't mind too much.

Could also make sense, but lead to confusion if multiple fields have the same type

Yes not sure about this, too, could be a bit confusing maybe if for most we pass str to url, but for Text we pass it to the text field. But still, I think i like passing the obvious ones, like tensor to tensor field etc, instead of only on main content field

But nothing automatic

What do u mean by this?

Signed-off-by: samsja <[email protected]>
@samsja
Copy link
Copy Markdown
Member Author

samsja commented Jan 26, 2023

but glad we aren't trying to do everything at the same time for once
we don't this at the BaseDocument level but tat at predefined document level. We don't come up with a generic solution but rather we decide for each Document

@github-actions github-actions bot added size/m and removed size/s labels Jan 26, 2023
Signed-off-by: samsja <[email protected]>
Signed-off-by: samsja <[email protected]>
Signed-off-by: samsja <[email protected]>
@samsja samsja changed the title feat: add validate on Text document feat: add validate function to documents to have less verbose api Jan 26, 2023
@anna-charlotte
Copy link
Copy Markdown
Contributor

For the Audio doc we could do the same, right? @samsja

@samsja
Copy link
Copy Markdown
Member Author

samsja commented Jan 26, 2023

For the Audio doc we could do the same, right? @samsja

I forgot this one

Signed-off-by: samsja <[email protected]>
@samsja samsja requested a review from anna-charlotte January 26, 2023 16:51
@github-actions
Copy link
Copy Markdown

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

@samsja samsja dismissed JohannesMessner’s stale review January 27, 2023 08:10

notification problem I guess

@samsja samsja merged commit 4311bcc into feat-rewrite-v2 Jan 27, 2023
@samsja samsja deleted the feat-validate-docs branch January 27, 2023 08:11
@samsja
Copy link
Copy Markdown
Member Author

samsja commented Jan 31, 2023

partially fix : #1056

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