Move MCPClient to root-level lib and manage optional dependencies #1238
Conversation
There was a problem hiding this comment.
Thanks for the catch.
However, I am afraid that with the current structure of the codebase, we cannot import MCPClient at the root level of the lib because mcp and mcpadapt are optional dependencies.
We should either fix the import to account for the optional nature of those dependencies, or fix the docs accordingly. In principle, I think it would be better to fix the import so the usage is consistent and neat.
|
It can be imported as follows: Example: https://github.com/sergiopaniego/samples/blob/main/smolagents_mcp.py |
|
@sergiopaniego Thanks, yep the issue isn't with being able to import it, but rather that the docs are incorrect about how to import it. @albertvillanova made an update so that the import error is supressed, this should address the optional dependency issue? |
There was a problem hiding this comment.
Thanks @sergiopaniego for chiming in and for your quick workaround: much appreciated. 🤗
And thank you @njbrake for the fast update!
Apologies for the confusion earlier: I realize I wasn't clear in my previous message. I didn't mean to suggest modifying the import at the root level. Rather, my intention was to handle the optional dependencies within the class __init__ method, as we already do for most of the models in models.py.
|
@albertvillanova ah, thanks for the clarification! Sorry I misunderstood you 😅 . I've made changes which I believe address your suggestion. Let me know what you think 👍 |
The readme instructions added in #1200
say to import MCPClient like
This PR is to add MCPClient to the
__init__so that the code in the readme is accurate