Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

docs: fix typo with Embedder service #269

Merged

Conversation

Iyadhfaleh
Copy link
Contributor

Hello ,

This PR is a suggestion to change the name of embedding providers variables and classes .

If you accept my suggestion , the PR to adopt this into the bundle is almost ready .

@Iyadhfaleh Iyadhfaleh force-pushed the fix/typo-embedder-to-embeddingProvider branch from 6db3e12 to 6659849 Compare March 29, 2025 11:30
@Iyadhfaleh Iyadhfaleh changed the title Fix typo fix: typo Mar 29, 2025
README.md Outdated
@@ -350,23 +350,23 @@ $eventDispatcher->addListener(ToolCallsExecuted::class, function (ToolCallsExecu
LLM Chain supports document embedding and similarity search using vector stores like ChromaDB, Azure AI Search, MongoDB
Atlas Search, or Pinecone.

For populating a vector store, LLM Chain provides the service `DocumentEmbedder`, which requires an instance of an
For populating a vector store, LLM Chain provides the service `EmbeddingProvider`, which requires an instance of an
Copy link
Member

Choose a reason for hiding this comment

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

this is the actual typo, right?

Suggested change
For populating a vector store, LLM Chain provides the service `EmbeddingProvider`, which requires an instance of an
For populating a vector store, LLM Chain provides the service `Embedder`, which requires an instance of an

The renaming from DocumentEmbedder to Embedder was intended and part of #140

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes , I reverted naming changes , I let just fix for the README file

Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

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

please decouple fixing the readme (valid and good catch) and renaming the Embedder (not wanted)

@Iyadhfaleh Iyadhfaleh force-pushed the fix/typo-embedder-to-embeddingProvider branch 3 times, most recently from c015069 to 916bd2d Compare March 29, 2025 14:10
@Iyadhfaleh Iyadhfaleh force-pushed the fix/typo-embedder-to-embeddingProvider branch from 916bd2d to dc3672a Compare March 29, 2025 14:12
@chr-hertel chr-hertel added the documentation Improvements or additions to documentation label Mar 29, 2025
@chr-hertel chr-hertel merged commit 375acf2 into php-llm:main Mar 29, 2025
6 of 7 checks passed
Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

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

Thanks, good catch! 👍

@chr-hertel chr-hertel changed the title fix: typo docs: fix typo with Embedder service Mar 29, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants