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

[docs]: update Redis (langchain-redis) documentation notebooks (vectorstore, llm caching, chat message history) #25113

Merged
merged 3 commits into from
Aug 22, 2024

Conversation

bsbodden
Copy link
Contributor

@bsbodden bsbodden commented Aug 6, 2024

  • Description: Adds notebooks for Redis Partner Package (langchain-redis)
  • Issue: N/A
  • Dependencies: None
  • Twitter handle: @bsbodden and @redis

@dosubot dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Aug 6, 2024
Copy link

vercel bot commented Aug 6, 2024

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

Name Status Preview Comments Updated (UTC)
langchain ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 22, 2024 3:50pm

@dosubot dosubot bot added Ɑ: vector store Related to vector store module 🔌: redis Primarily related to Redis integrations 🤖:docs Changes to documentation and examples, like .md, .rst, .ipynb files. Changes to the docs/ folder labels Aug 6, 2024
Copy link
Collaborator

@ccurme ccurme left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for doing this. Some comments / questions:

  • We recently introduced an effort to standardize our integration docs, including vector store docs. See issue here: Standardize vector store docs #24800. This PR recently standardized the redis vector store doc. We are un-doing some of that here.
  • If possible we should update the existing caching doc instead of writing a new one for Redis.
  • The changes to the vector store doc in particular remove what I'd consider useful sections (e.g., overview of deployment options, deploying via Docker, etc.). I'm wondering what is the motivation for deleting them. Left some comments in-line.

@bsbodden
Copy link
Contributor Author

bsbodden commented Aug 7, 2024

@ccurme some answers...

Looks good, thanks for doing this. Some comments / questions:

  • We recently introduced an effort to standardize our integration docs, including vector store docs. See issue here: Standardize vector store docs #24800. This PR recently standardized the redis vector store doc. We are un-doing some of that here.

Sure, I'll check it the PR and adjust

  • If possible we should update the existing caching doc instead of writing a new one for Redis.

Speaking with @efriis about the caching page... We have enough caches that there should be individual pages. For now create a caches folder with just redis (I’ll deal with the rest)

  • The changes to the vector store doc in particular remove what I'd consider useful sections (e.g., overview of deployment options, deploying via Docker, etc.). I'm wondering what is the motivation for deleting them. Left some comments in-line.

I particularly felt there was too much ceremony before getting to the usage of the library, but I can bring some of that back.

@bsbodden
Copy link
Contributor Author

Looks good, thanks for doing this. Some comments / questions:

  • We recently introduced an effort to standardize our integration docs, including vector store docs. See issue here: Standardize vector store docs #24800. This PR recently standardized the redis vector store doc. We are un-doing some of that here.
  • If possible we should update the existing caching doc instead of writing a new one for Redis.
  • The changes to the vector store doc in particular remove what I'd consider useful sections (e.g., overview of deployment options, deploying via Docker, etc.). I'm wondering what is the motivation for deleting them. Left some comments in-line.

Looked at all the associated issues with standardizing the vectorstores docs, and modified the redis notebook to follow the guidelines. Another PR for the langchain-redis project with updated DocStrings will follow. Cheers.

@tylerhutcherson
Copy link
Contributor

LGTM from a Reids standpoint -- thanks @bsbodden !

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Aug 22, 2024
@ccurme ccurme mentioned this pull request Aug 22, 2024
1 task
@ccurme ccurme merged commit 29c873d into langchain-ai:master Aug 22, 2024
13 checks passed
@bsbodden bsbodden deleted the redis/langchain-redis-docs branch August 30, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:docs Changes to documentation and examples, like .md, .rst, .ipynb files. Changes to the docs/ folder lgtm PR looks good. Use to confirm that a PR is ready for merging. 🔌: redis Primarily related to Redis integrations size:XXL This PR changes 1000+ lines, ignoring generated files. Ɑ: vector store Related to vector store module
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants