-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add python binding for vectorstore #573
base: master
Are you sure you want to change the base?
Conversation
gel/ai/vectorstore.py
Outdated
# Mark which fields were actually passed by the user (ignore 'id'). | ||
# So if user calls Record(id=..., text=None), "text" will appear here. | ||
object.__setattr__(self, "_explicitly_set_fields", set(kwargs.keys())) |
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.
This can also be done by wrapping the constructor with a Python function.
gel/ai/vectorstore.py
Outdated
Record( | ||
id=result.id, | ||
text=result.text, | ||
embedding=result.embedding and list(result.embedding), |
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.
why wrapping with list()
here?
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.
I think I had some type inconsistence, will see what happens after I update things
gel/ai/vectorstore.py
Outdated
id=result.id, | ||
text=result.text, | ||
embedding=result.embedding and list(result.embedding), | ||
metadata=(json.loads(result.metadata)), |
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.
result.metadata
can beNone
- redundant parentheses
gel/ai/vectorstore.py
Outdated
metadata := <json>$metadata, | ||
} | ||
select ( | ||
insert {{record_type}} { |
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.
Composing EdgeQL like this would need quoting, see edb.edgeql.quote.quote_ident()
in the server code.
Metadata filter should be further developed and tested. The castings are not correct in some cases, I don't have knowledge atm to anticipate all possible use cases for metadata filtering. I left comments where things should be further updated/developed.