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

Make callbacks available at the main module level again, but lazily #277

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

ramnes
Copy link
Contributor

@ramnes ramnes commented Aug 14, 2023

This keeps the imports fast, but with a few advantages over the current implementation:

  1. we don't introduce a breaking change to the end users;
  2. it's easier to import chainlit once and getting all things from there;
  3. doing so also makes it easier to read the code and understand what comes from chainlit: cl.LangchainCallbackHandler makes it obvious that it's Chainlit's callback handler for Langchain (as opposed to LangchainCallbackHandler, without cl. in front).

@ramnes ramnes added the enhancement New feature or request label Aug 14, 2023
@ramnes ramnes force-pushed the gg/module-getattr branch 2 times, most recently from 473b236 to 5e7719c Compare August 14, 2023 10:39
This keeps the imports fast, but with a few advantages over the
current implementation:

1. we don't introduce a breaking change to the end users;
2. it's easier to import `chainlit` once and getting all things from
there;
3. doing so also makes it easier to read the code and understand what
comes from chainlit: `cl.LangchainCallbackHandler` makes it obvious
that it's Chainlit's callback handler for Langchain (as opposed to
`LangchainCallbackHandler`, without `cl.` in front).
@@ -181,6 +181,15 @@ def sleep(duration: int):
return asyncio.sleep(duration)


__getattr__ = make_module_getattr(
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I'm new in python, I wonder why not just import the handlers like this:

from chainlit.langchain.callbacks import LangchainCallbackHandler, AsyncLangchainCallbackHandler, LlamaIndexCallbackHandler, HaystackAgentCallbackHandler

Does it has any advantages I didn't know?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be working fine but would also import the Langchain library by default, which we want to avoid for performance reasons. This PR will make the Langchain (and other libraries) imports lazy.

@ramnes ramnes merged commit 80e2aba into main Aug 16, 2023
@alimtunc alimtunc deleted the gg/module-getattr branch August 16, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants