refactor: switch langchain imports to core #805
Conversation
|
|
jan-kubica
force-pushed
the
feat/fix-langchain-import
branch
3 times, most recently
from
ebd440d to
4bbad21
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Disclaimer: Experimental PR review
PR Summary
- Updated langchain imports to use
langchain_core - Switched
langchaindependency tolangchain-coreinpyproject.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
jan-kubica
force-pushed
the
feat/fix-langchain-import
branch
2 times, most recently
from
229f45b to
e17e5ab
Compare
jan-kubica
changed the title
jan-kubica
force-pushed
the
feat/fix-langchain-import
branch
from
e17e5ab to
14440b4
Compare
jan-kubica
force-pushed
the
feat/fix-langchain-import
branch
from
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.
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.
If yes, is there a way to make this backwards compatible?
There was a problem hiding this comment.
-
Users should install
langfusewith thelangchainextra (and this will ensure they have thelangchain-corepackage installed in their env). If they also havelangchainpackage v0.0.310 installed explicitly or for some other reason, it's fine, these are not clashing. -
However, if users do not install
langfusewith thelangchainextra and do not havelangchain-corein 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.
bump on this? it's a minor inconvenience, but I'd love to see it merged
There was a problem hiding this comment.
32 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| @@ -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.
logic: langchain-core is marked as optional but appears to be required by several other dependencies. Consider making it a required dependency if it's needed for core functionality.
There was a problem hiding this comment.
LGTM
7 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
Closes langfuse/langfuse#1789
Greptile Summary
Disclaimer: Experimental PR review
This PR modernizes the Langfuse Python SDK by refactoring LangChain imports and adding new sampling functionality. Here's a concise summary:
langchaintolangchain_coreacross the codebase for better modularitySampler.pywith SHA-256 based deterministic sampling to control data volumepyproject.tomldependencies to uselangchain-coreand related packageslangchain.pyfor IBM Watson.ai and other providersKey points:
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!