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

langchain[minor]: Add native async implementation to LLMFilter, add concurrency to both sync and async paths #22739

Merged
merged 9 commits into from
Jun 11, 2024

Conversation

pprados
Copy link
Contributor

@pprados pprados commented Jun 10, 2024

Thank you for contributing to LangChain!

  • PR title: "langchain: Fix chain_filter.py to be compatible with async"

  • PR message:

    • Description: chain_filter is not compatible with async.
    • Twitter handle: pprados
  • [X ] Lint and test: Run make format, make lint and make test from the root of the package(s) you've modified. See contribution guidelines for more: https://python.langchain.com/docs/contributing/

Copy link

vercel bot commented Jun 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Jun 11, 2024 6:34am

@pprados pprados marked this pull request as ready for review June 10, 2024 15:31
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. 🤖:improvement Medium size change to existing code to handle new use-cases labels Jun 10, 2024
Copy link
Collaborator

@eyurtsev eyurtsev left a comment

Choose a reason for hiding this comment

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

Would you mind requesting a re-review when ready? (I'm using Request Changes to make it easier for me to discover if a PR is waiting on me or on the PR author for action.)

@eyurtsev
Copy link
Collaborator

@pprados if you're able try to use [patch], [minor], [major] in the PR message it makes it easier to see where a review is required + we'll be using it in change logs down the road (This looks like [minor] to me rather than a bug fix since it's adding a native async implementation.)

@eyurtsev eyurtsev changed the title Fix chain_filter.py to be compatible with async langchain[minor]: Add native async implementation to LLMFilter, add concurrency Jun 10, 2024
@eyurtsev eyurtsev changed the title langchain[minor]: Add native async implementation to LLMFilter, add concurrency langchain[minor]: Add native async implementation to LLMFilter, add concurrency to both sync and async Jun 10, 2024
@eyurtsev eyurtsev changed the title langchain[minor]: Add native async implementation to LLMFilter, add concurrency to both sync and async langchain[minor]: Add native async implementation to LLMFilter, add concurrency to both sync and async paths Jun 10, 2024
@eyurtsev eyurtsev self-assigned this Jun 10, 2024
prakul and others added 7 commits June 10, 2024 18:25
**Description:**: Update MongoDB information in llm_caching
langchain-ai#22707)

**Description:** Scheduled GitHub Actions to run only on the upstream
repository

**Issue:** Fixes langchain-ai#22706 

**Twitter handle:** @coolbeevip
…elf query retriever (langchain-ai#22678)

The fact that we outsourced pgvector to another project has an
unintended effect. The mapping dictionary found by
`_get_builtin_translator()` cannot recognize the new version of pgvector
because it comes from another package.
`SelfQueryRetriever` no longer knows `PGVector`.

I propose to fix this by creating a global dictionary that can be
populated by various database implementations. Thus, importing
`langchain_postgres` will allow the registration of the `PGvector`
mapping.

But for the moment I'm just adding a lazy import

Furthermore, the implementation of _get_builtin_translator()
reconstructs the BUILTIN_TRANSLATORS variable with each invocation,
which is not very efficient. A global map would be an optimization.

- **Twitter handle:** pprados

@eyurtsev, can you review this PR? And unlock the PR [Add async mode for
pgvector](langchain-ai/langchain-postgres#32)
and PR [community[minor]: Add SQL storage
implementation](langchain-ai#22207)?

Are you in favour of a global dictionary-based implementation of
Translator?
Signed-off-by: zhangwangda <zhangwangda94@163.com>
Hi 👋 

First off, thanks a ton for your work on this 💚 Really appreciate what
you're providing here for the community.

## Description

This PR adds a basic language parser for the
[Elixir](https://elixir-lang.org/) programming language. The parser code
is based upon the approach outlined in
langchain-ai#13318: it's using
`tree-sitter` under the hood and aligns with all the other `tree-sitter`
based parses added that PR.

The `CHUNK_QUERY` I'm using here is probably not the most sophisticated
one, but it worked for my application. It's a starting point to provide
"core" parsing support for Elixir in LangChain. It enables people to use
the language parser out in real world applications which may then lead
to further tweaking of the queries. I consider this PR just the ground
work.

- **Dependencies:** requires `tree-sitter` and `tree-sitter-languages`
from the extended dependencies
- **Twitter handle:**`@bitcrowd`

## Checklist

- [x] **PR title**: "package: description"
- [x] **Add tests and docs**
- [x] **Lint and test**: Run `make format`, `make lint` and `make test`
from the root of the package(s) you've modified.

<!-- If no one reviews your PR within a few days, please @-mention one
of baskaryan, efriis, eyurtsev, ccurme, vbarda, hwchase17. -->
@efriis efriis self-assigned this Jun 11, 2024
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jun 11, 2024
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Jun 11, 2024
@pprados
Copy link
Contributor Author

pprados commented Jun 11, 2024

@eyurtsev
can you review this PR please ?

@pprados pprados requested a review from eyurtsev June 11, 2024 06:42
_input = self.get_input(query, doc)
output_dict = self.llm_chain.invoke(_input, config={"callbacks": callbacks})

config = RunnableConfig(callbacks=callbacks)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be worthwhile to update the docs to say that this runs without a limit on the max concurrency. This could lead to issues for some users that will be difficult to debug.

We'll need to make a change here to expose config in the signature instead of callbacks, but that's not for this PR

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Jun 11, 2024
@eyurtsev eyurtsev merged commit 0908b01 into langchain-ai:master Jun 11, 2024
60 checks passed
@pprados pprados deleted the pprados/fix_chain_filter branch June 18, 2024 06:33
hinthornw pushed a commit that referenced this pull request Jun 20, 2024
…oncurrency to both sync and async paths (#22739)

Thank you for contributing to LangChain!

- [ ] **PR title**: "langchain: Fix chain_filter.py to be compatible
with async"


- [ ] **PR message**: 
    - **Description:** chain_filter is not compatible with async.
    - **Twitter handle:** pprados


- [X ] **Lint and test**: Run `make format`, `make lint` and `make test`
from the root of the package(s) you've modified. See contribution
guidelines for more: https://python.langchain.com/docs/contributing/

---------

Signed-off-by: zhangwangda <zhangwangda94@163.com>
Co-authored-by: Prakul <discover.prakul@gmail.com>
Co-authored-by: Lei Zhang <zhanglei@apache.org>
Co-authored-by: Gin <ictgtvt@gmail.com>
Co-authored-by: wangda <38549158+daziz@users.noreply.github.com>
Co-authored-by: Max Mulatz <klappradla@posteo.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:improvement Medium size change to existing code to handle new use-cases lgtm PR looks good. Use to confirm that a PR is ready for merging. size:M This PR changes 30-99 lines, ignoring generated files. template
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants