New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: adding field descriptions to predefined text document #1770
docs: adding field descriptions to predefined text document #1770
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1770 +/- ##
==========================================
+ Coverage 83.73% 83.87% +0.13%
==========================================
Files 136 136
Lines 9040 9041 +1
==========================================
+ Hits 7570 7583 +13
+ Misses 1470 1458 -12
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
@JoanFM I've added an initial implementation of the change to add a field description for attribute of |
Hello @punndcoder28 , Thanks for the contribution. However, we would need you to sign off the commit so that we can incorporate your improvements. |
fe2cae2
to
41ee465
Compare
Sorry for that. The commits should be signed off now |
4146e00
to
213448a
Compare
@JoanFM What is the correct github flow? Should I rebase and keep my commits for a PR to a single commit? Or each change should be a different commit? |
856b6ea
to
81b7fdc
Compare
81b7fdc
to
0b4f618
Compare
As you want, eventually all the commits will be squashed into a single one, so your choice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! :)
0b4f618
to
f7362a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A minor change
docarray/documents/text.py
Outdated
) | ||
embedding: Optional[AnyEmbedding] = Field( | ||
description='Store an embedding: a vector representation of the text', | ||
example='''[[1, 1, 1], [1, 0, 1], [0, 0, 1]]''', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better have a single vector in the example
To resolve the conflicts, make sure to add None as the default value for the Optional fields. |
Also, the embedding example can be a list and should not be actually a text |
Signed-off-by: punndcoder28 <puneethk.2899@gmail.com>
Signed-off-by: punndcoder28 <puneethk.2899@gmail.com>
f7362a6
to
6e80337
Compare
Signed-off-by: punndcoder28 <puneethk.2899@gmail.com>
6e80337
to
2ee9c01
Compare
Text