langchain-ai / langchain-elastic

Elasticsearch integration into LangChain
MIT License
44 stars 14 forks source link

Allow query embeddings to be passed in max_marginal_relevance_search #40

Open rishabh208gupta opened 2 months ago

rishabh208gupta commented 2 months ago

As part of https://github.com/elastic/elasticsearch-py/issues/2620 , elasticsearch store mmr search allows query or query_embeddings optionally. It would be nice to have it in https://github.com/langchain-ai/langchain-elastic/blob/19cb4ab322ea185aef26034f12a8178fbe0bbef0/libs/elasticsearch/langchain_elasticsearch/vectorstores.py#L882 as well.

P.S. Can I work on it?

pquentin commented 2 months ago

Thanks for your interest @rishabh208gupta! Sure, you can work on this 👍

rishabh208gupta commented 2 months ago

Hi @pquentin, is there any document I can follow to run the tests on local?

pquentin commented 2 months ago

I never worked on this repo before, and it does not look like running tests is documented. You can try reproducing the steps in https://github.com/langchain-ai/langchain-elastic/blob/main/.github/workflows/_test.yml

rishabh208gupta commented 2 months ago

Okay thanks :)

miguelgrinberg commented 2 months ago

Information for contributors isn't currently available, this is something that we plan to improve. But in the meantime, you can run the test suite locally from the lib/elasticsearch directory:

poetry install --with lint,test
make lint
make test
make integration_tests
rishabh208gupta commented 2 months ago

I realise that to make this change, we would need to change the method definition in https://github.com/langchain-ai/langchain/blob/b28bc252c4e7058951c9d3be8ee81595029eab83/libs/core/langchain_core/vectorstores/base.py#L679C9-L679C38

miguelgrinberg commented 2 months ago

@rishabh208gupta as long as it changes in a backwards compatible manner, it is not a problem.