Skip to content
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

experimental: LLMGraphTransformer - added relationship properties. #21856

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

istvan-nebulinq
Copy link

  • Description:
    The generated relationships in the graph had no properties, but the Relationship class was properly defined with properties. This made it very difficult to transform conditional sentences into a graph. Adding properties to relationships can solve this issue elegantly.
    The changes expand on the existing LLMGraphTransformer implementation but add the possibility to define allowed relationship properties like this: LLMGraphTransformer(llm=llm, relationship_properties=["Condition", "Time"],)
  • Issue:
    no issue found
  • Dependencies:
    n/a
  • Twitter handle:
    @IstvanSpace

-Quick Test

from dotenv import load_dotenv
import os
from langchain_community.graphs import Neo4jGraph
from langchain_experimental.graph_transformers import LLMGraphTransformer
from langchain_openai import ChatOpenAI
from langchain_core.prompts import ChatPromptTemplate
from langchain_core.documents import Document

load_dotenv()
os.environ["NEO4J_URI"] = os.getenv("NEO4J_URI")
os.environ["NEO4J_USERNAME"] = os.getenv("NEO4J_USERNAME")
os.environ["NEO4J_PASSWORD"] = os.getenv("NEO4J_PASSWORD")
graph = Neo4jGraph()
llm = ChatOpenAI(temperature=0, model_name="gpt-4o")
llm_transformer = LLMGraphTransformer(llm=llm)
#text = "Harry potter likes pies, but only if it rains outside"
text = "Jack has a dog named Max. Jack only walks Max if it is sunny outside."
documents = [Document(page_content=text)]
llm_transformer_props = LLMGraphTransformer(
llm=llm,
relationship_properties=["Condition"],
)
graph_documents_props = llm_transformer_props.convert_to_graph_documents(documents)
print(f"Nodes:{graph_documents_props[0].nodes}")
print(f"Relationships:{graph_documents_props[0].relationships}")
graph.add_graph_documents(graph_documents_props)

…LLMGraphTransformer(llm=llm, relationship_properties=["Condition", "Time"],)
Copy link

vercel bot commented May 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview May 31, 2024 5:25pm

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. 🔌: neo4j Primarily related to Neo4j integrations 🤖:improvement Medium size change to existing code to handle new use-cases labels May 18, 2024
@baskaryan
Copy link
Collaborator

cc @tomasonjo

@tomasonjo
Copy link
Contributor

LGTM... can you add node and relationship property description to docstring?

@tomasonjo
Copy link
Contributor

@istvan-nebulinq can you please fix lint errors and I can add docstring later if needed

@istvan-nebulinq
Copy link
Author

Yes. Will do asap.

@istvan-nebulinq
Copy link
Author

@tomasonjo done. passed my local tests (now including lint). synced with main as well.

@tomasonjo
Copy link
Contributor

@istvan-nebulinq thank you! Cc @baskaryan

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label May 27, 2024
@baskaryan baskaryan enabled auto-merge (squash) May 27, 2024 20:53
@tomasonjo
Copy link
Contributor

@istvan-nebulinq can you run

make format

@istvan-nebulinq
Copy link
Author

make format

Executed the command in the root dir. Here is the output:
vscode ➜ /workspaces/langchain (graphtransformer_fix) $ make format
poetry run ruff format docs templates cookbook
1403 files left unchanged
poetry run ruff check --select I --fix docs templates cookbook
All checks passed!

@tomasonjo
Copy link
Contributor

tomasonjo commented May 30, 2024 via email

auto-merge was automatically disabled May 31, 2024 01:06

Head branch was pushed to by a user without write access

@istvan-nebulinq
Copy link
Author

Try executing in libs/experimental V čet., 30. maj 2024, 12:56 je oseba istvan-nebulinq < @.> napisala:

make format Executed the command in the root dir. Here is the output: vscode ➜ /workspaces/langchain (graphtransformer_fix) $ make format poetry run ruff format docs templates cookbook 1403 files left unchanged poetry run ruff check --select I --fix docs templates cookbook All checks passed! — Reply to this email directly, view it on GitHub <#21856 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEYGGTIX3WOLVWNXTVUCUMDZE6AAHAVCNFSM6AAAAABH46WLAOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBQG43DSOJYGI . You are receiving this because you were mentioned.Message ID: @.
>

it worked. I've pushed the changes in that one file.
FYI: it also modified a lot of other files. mainly added an empty second line. trace:

vscode ➜ /workspaces/langchain/libs/experimental (graphtransformer_fix) $ make format
poetry run ruff format .
47 files reformatted, 154 files left unchanged
poetry run ruff --select I --fix .
warning: ruff <path> is deprecated. Use ruff check <path> instead.
All checks passed!

vscode ➜ /workspaces/langchain/libs/experimental (graphtransformer_fix) $ make format
poetry run ruff format .
201 files left unchanged
poetry run ruff --select I --fix .
warning: ruff <path> is deprecated. Use ruff check <path> instead.
All checks passed!

@tomasonjo
Copy link
Contributor

@istvan-nebulinq
One last thing... since we are using dynamic classes the linter is not happy...
Can you add # type: ignore at the end of line 423. You can see example in line 422

Sorry for the trouble, and then make sure to rerun the format again

@istvan-nebulinq
Copy link
Author

@tomasonjo it's all good. thanks for your patience while I get up to speed with the flow.
I've written a unit test for this, however, it obviously needs an openai api key to make it work. do you have any policy around this situation?

@tomasonjo
Copy link
Contributor

So those type of tests that require API keys or connection to database are placed under integration tests

@tomasonjo
Copy link
Contributor

cc @baskaryan

@baskaryan
Copy link
Collaborator

So those type of tests that require API keys or connection to database are placed under integration tests

yep. that or ideally you mock out the model call and keep it a unit test, since its not openai behavior that this test should be checking

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:improvement Medium size change to existing code to handle new use-cases lgtm PR looks good. Use to confirm that a PR is ready for merging. 🔌: neo4j Primarily related to Neo4j integrations size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants