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

Redis Vectorstore: Redis.from_texts_return_keys() got multiple values for argument 'cls' #4896

Closed
2 of 14 tasks
iamadhee opened this issue May 18, 2023 · 6 comments
Closed
2 of 14 tasks

Comments

@iamadhee
Copy link
Contributor

iamadhee commented May 18, 2023

System Info

Python 3.10.4
langchain==0.0.171
redis==3.5.3
redisearch==2.1.1

Who can help?

@tylerhutcherson

Information

  • The official example notebooks/scripts
  • My own modified scripts

Related Components

  • LLMs/Chat Models
  • Embedding Models
  • Prompts / Prompt Templates / Prompt Selectors
  • Output Parsers
  • Document Loaders
  • Vector Stores / Retrievers
  • Memory
  • Agents / Agent Executors
  • Tools / Toolkits
  • Chains
  • Callbacks/Tracing
  • Async

Reproduction

I was able to override issue #3893 by temporarily disabling the _check_redis_module_exist, post which I'm getting the below error when calling the from_texts_return_keys within the from_documents method in Redis class. Seems the argument cls is not needed in the from_texts_return_keys method, since it is already defined as a classmethod.

File "/workspaces/chatdataset_backend/adapters.py", line 96, in load
    vectorstore = self.rds.from_documents(documents=documents, embedding=self.embeddings)
  File "/home/codespace/.python/current/lib/python3.10/site-packages/langchain/vectorstores/base.py", line 296, in from_documents
    return cls.from_texts(texts, embedding, metadatas=metadatas, **kwargs)
  File "/home/codespace/.python/current/lib/python3.10/site-packages/langchain/vectorstores/redis.py", line 448, in from_texts
    instance, _ = cls.from_texts_return_keys(
TypeError: Redis.from_texts_return_keys() got multiple values for argument 'cls'

Expected behavior

Getting rid of cls argument from all the Redis class methods wherever required. Was able to solve the issue with this fix.

@tylerhutcherson
Copy link
Contributor

This is a high priority issue. I ran into this myself yesterday.

Thanks for creating the issue.

@iamadhee
Copy link
Contributor Author

Thanks, @tylerhutcherson If you could assign this to me, I can solve it along with #4899 and merge it.

@tylerhutcherson
Copy link
Contributor

tylerhutcherson commented May 18, 2023

Yes, please do. Add unit tests to catch similar issues if you can. Tag me in the PR review!

@tylerhutcherson
Copy link
Contributor

@iamadhee FYI #4

@iamadhee
Copy link
Contributor Author

@tylerhutcherson Have added #4932

@dosubot
Copy link

dosubot bot commented Sep 15, 2023

Hi, @iamadhee! I'm Dosu, and I'm helping the LangChain team manage their backlog. I wanted to let you know that we are marking this issue as stale.

From what I understand, the issue you reported was about the from_texts_return_keys() method in the Redis class receiving multiple values for the cls argument. Tylerhutcherson confirmed the issue and suggested removing the cls argument from all the Redis class methods where it is not needed. You agreed to solve the issue and merge it along with another one. Tylerhutcherson asked for unit tests and requested to be tagged in the PR review. You added a PR and updated Tylerhutcherson.

Before we close this issue, we wanted to check if it is still relevant to the latest version of the LangChain repository. If it is, please let us know by commenting on the issue. Otherwise, feel free to close the issue yourself, or it will be automatically closed in 7 days.

Thank you for your contribution to the LangChain repository!

@dosubot dosubot bot added the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Sep 15, 2023
@dosubot dosubot bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 22, 2023
@dosubot dosubot bot removed the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Sep 22, 2023
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