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

Langsmith does not properly declare dependencies on langchain #1339

Open
cosinequanon opened this issue Dec 13, 2024 · 3 comments
Open

Langsmith does not properly declare dependencies on langchain #1339

cosinequanon opened this issue Dec 13, 2024 · 3 comments

Comments

@cosinequanon
Copy link

Feature request

The langsmith python package should declare what dependencies it needs in the pyproject.toml file. Specifically I see no mention of langchain in https://github.com/langchain-ai/langsmith-sdk/blob/main/python/pyproject.toml#L27-L77 but I see it used here

from langchain.evaluation.schema import StringEvaluator # noqa: F811

It should be listed as an optional dependency in some way

Motivation

It would be easier to use this package if it declared all of the dependencies correctly. I ran into an issue where I thought I had installed the package and dependencies correctly but it failed when I used LangChainStringEvaluator which was unexpected.

@hinthornw
Copy link
Collaborator

hinthornw commented Dec 13, 2024

We cannot declare a dependency on LangChain because the dependency is cyclic. Is it surprising that an Optional LangChainStringEvaluator would have that dep? These classes that use langchain are optional entrypoints for specific evaluation logic that uses langchain and aren't in the hot path of >95% of usage. In a strict deptree world without dynamic imports, this would be a third package, but that creates another hurdle for people to install

@cosinequanon
Copy link
Author

For context, this isn't just a minor inconvenience where I have to manually add a package. We use bazel for tracking python dependencies so it breaks when using this piece of code, since the dependencies are automatically managed by gazelle, people who use the system aren't used to running into this type of error. It's really annoying to have to educate users how to work around this.

I would encourage you to split out the functionality into it's own package to break the cycle. While it might be a hurdle at installation, at least you aren't importing things you aren't declaring. That practice seems much worse since it breaks the contract people have when installing a library

@hinthornw
Copy link
Collaborator

Thanks for the context, and for reporting - will discuss it with the team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants