Adds GenAI client object #1
Conversation
neo4j_genai/src/client.py
Outdated
| List[Dict[str, Any]]: List of dictionaries containing the query results. | ||
| """ | ||
| params = params or {} | ||
| with driver.session() as session: |
There was a problem hiding this comment.
What is the proper usage for the driver? I have seen in other places where the database needs to be passed to session e.g.
with driver.session(database="neo4j") as session:
Is this necessarily the case?
neo4j_genai/src/client.py
Outdated
| List[Dict[str, Any]]: List of dictionaries containing the query results. | ||
| """ | ||
| params = params or {} | ||
| with driver.session() as session: |
There was a problem hiding this comment.
You are passing the driver all around the code. Why not just set it in the __init__ and do with self.driver.session() here?
There was a problem hiding this comment.
I think we initially decided this during the design phase so that it is more explicit, right? I do think setting in __init__ is more readable and Pythonic though. I would prefer to do it like you suggested here
|
@jonbesga Oskar and I discussed earlier today that we want to add this behaviour for allowing users to give more "advanced" queries where they also pass in a Cypher query string as seen in:
I don't think this should be part of this PR, but I think it's good to include this in the first version of this package, what do you think? |
I'm confused what behaviour are your referring to😅 |
src/neo4j_genai/types.py
Outdated
| @root_validator(pre=True) | ||
| def check_query(cls, values): | ||
| query_vector, query_text = values.get("query_vector"), values.get("query_text") | ||
| if bool(query_vector) ^ bool(query_text): |
There was a problem hiding this comment.
The ^ operator only works with booleans
|
Can you squash all the commits into one? |
I've now set the default for merging PRs to be 'Squash and merge', so on the base branch it will be squashed |
| name: str | ||
| label: str | ||
| property: str | ||
| dimensions: int = Field(ge=1, le=2048) |
There was a problem hiding this comment.
This will change to 4096 in the next neo4j v5.18 release, so we will need to make this change in future
| Validates that one of either query_vector or query_text is provided exclusively. | ||
| """ | ||
| query_vector, query_text = values.get("query_vector"), values.get("query_text") | ||
| if not (bool(query_vector) ^ bool(query_text)): |
There was a problem hiding this comment.
The ^ Python operator only works for booleans
| def __init__( | ||
| self, | ||
| driver: Driver, | ||
| embeddings: Optional[Embeddings] = None, |
There was a problem hiding this comment.
Consider embedding instead of embeddings? This could be confusing because it could mean embedding the noun instead of the verb
There was a problem hiding this comment.
Embedder as seen in haystack
There was a problem hiding this comment.
Both class and argument
| Returns: | ||
| List[Dict[str, Any]]: List of dictionaries containing the query results. | ||
| """ | ||
| with self.driver.session() as session: |
There was a problem hiding this comment.
Use execute_query as seen in https://neo4j.com/docs/api/python-driver/current/
Description
Adds
GenAIClientto create indexes, drops indexes, and perform similarity search on a Neo4j database. Also contains pre-commit configuration.Checklist
Configure pre-commit-configHandled in Jonbesga/tests #2Testing
Unit tests passTests are on another PR (Jonbesga/tests #2)