-
Notifications
You must be signed in to change notification settings - Fork 104
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
refactor: switch langchain imports to core #805
base: main
Are you sure you want to change the base?
Conversation
ebd440d
to
4bbad21
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.
Disclaimer: Experimental PR review
PR Summary
- Updated langchain imports to use
langchain_core
- Switched
langchain
dependency tolangchain-core
inpyproject.toml
- Modified import statements in test files to use updated packages
- Updated error message for missing langchain installation
- Changed Bedrock model import to use
langchain_aws
6 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
229f45b
to
e17e5ab
Compare
e17e5ab
to
14440b4
Compare
14440b4
to
d754d09
Compare
@maxdeichmann could you please review? thank you |
@@ -13,7 +13,7 @@ pydantic = ">=1.10.7, <3.0" | |||
backoff = ">=1.10.0" | |||
openai = { version = ">=0.27.8", optional = true } | |||
wrapt = "^1.14" | |||
langchain = { version = ">=0.0.309", optional = true } | |||
langchain-core = { version = ">=0.2.0", optional = true } |
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 a lot for the contribution!
Do you know how backwards compatible this is? We might have users who explicitly installed langchain version 0.0.310. I assume that the upgrade would not work for them, correct?
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.
If yes, is there a way to make this backwards compatible?
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.
-
Users should install
langfuse
with thelangchain
extra (and this will ensure they have thelangchain-core
package installed in their env). If they also havelangchain
package v0.0.310 installed explicitly or for some other reason, it's fine, these are not clashing. -
However, if users do not install
langfuse
with thelangchain
extra and do not havelangchain-core
in the env for some other reason, the upgrade will not work for them. I would say that is expected and correct though (if you don't specify you want to use langfuse with langchain, then langfuse<>langchain might not be compatible).
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.
bump on this? it's a minor inconvenience, but I'd love to see it merged
Closes langfuse/langfuse#1789